[PATCH 00/14] drm/amdgpu: introduce secure buffer object support (trusted memory zone)

[PATCH 00/14] drm/amdgpu: introduce secure buffer object support (trusted memory zone)

[Huang, Ray]

Hi all,

These series of patches introduce a feature to support secure buffer object.
The Trusted Memory Zone (TMZ) is a method to protect the contents being written
to and read from memory. We use TMZ hardware memory protection scheme to
implement the secure buffer object support.

TMZ is the page-level protection that hardware will detect the TMZ bit in the
page table entry to set the current page is encrypted. With this hardware
feature, we design a BO-level protection in kernel driver to provide a new flag
AMDGPU_GEM_CREATE_ENCRYPTED to gem create ioctl to libdrm for the secure buffer
allocation. And also provide the AMDGPU_CTX_ALLOC_FLAGS_SECURE to indicate the
context is trusted or not. If the BO is secure, then the data is encrypted, only
the trusted IP blocks such as gfx, sdma, vcn are able to decrypt. CPU as the
un-trusted IP are unable to read the secure buffer.

We will submit the new secure context interface later for libdrm, and create a
new test suite to verify the security feature in the libdrm unit tests.

Suite id = 11: Name 'Security Tests status: ENABLED'
Test id 1: Name: 'allocate secure buffer test status: ENABLED'
Test id 2: Name: 'graphics command submission under secure context status: ENABLED'

Thanks,
Ray

Alex Deucher (4):
  drm/amdgpu: add UAPI for creating encrypted buffers
  drm/amdgpu: add UAPI for creating secure contexts (v2)
  drm/amdgpu: define the TMZ bit for the PTE
  drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

Huang Rui (10):
  drm/amdgpu: add tmz feature parameter (v2)
  drm/amdgpu: add amdgpu_tmz data structure
  drm/amdgpu: add function to check tmz capability (v4)
  drm/ttm: add helper to get buffer object with ttm_mem_reg
  drm/amdgpu: revise the function to allocate secure context (v2)
  drm/amdgpu: add tmz bit in frame control packet
  drm/amdgpu: expand the emit tmz interface with trusted flag
  drm/amdgpu: expand the context control interface with trust flag
  drm/amdgpu: set trusted mode while the job is under secure context
    (v2)
  drm/amdgpu: modify the method to use mem under buffer object for
    amdgpu_ttm_tt_pte_flags

 drivers/gpu/drm/amd/amdgpu/Makefile        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  7 ++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c    | 19 +++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h    |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 11 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c     |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h    |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  9 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c    | 49 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h    | 39 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 23 +++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  3 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     | 20 +++++++++---
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c      |  3 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c      |  3 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  3 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 16 +++++++---
 drivers/gpu/drm/amd/amdgpu/nvd.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/soc15d.h        |  1 +
 include/drm/ttm/ttm_bo_driver.h            | 13 ++++++++
 include/uapi/drm/amdgpu_drm.h              |  9 +++++-
 25 files changed, 230 insertions(+), 34 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h

-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/14] drm/amdgpu: add UAPI for creating encrypted buffers

[Huang, Ray]

From: Alex Deucher <alexander.deucher@amd.com>

Add a flag to the GEM_CREATE ioctl to create encrypted buffers.
Buffers with this flag set will be created with the TMZ bit set
in the PTEs or engines accessing them.  This is required in order
to properly access the data from the engines.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index f3ad429..f90b453 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -135,6 +135,11 @@ extern "C" {
  * releasing the memory
  */
 #define AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE	(1 << 9)
+/* Flag that BO will be encrypted and that the TMZ bit should be
+ * set in the PTEs when mapping this buffer via GPUVM or
+ * accessing it with various hw blocks
+ */
+#define AMDGPU_GEM_CREATE_ENCRYPTED		(1 << 10)
 
 struct drm_amdgpu_gem_create_in  {
 	/** the requested memory size */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/14] drm/amdgpu: add UAPI for creating secure contexts (v2)

[Huang, Ray]

From: Alex Deucher <alexander.deucher@amd.com>

Add a flag for when allocating a context to flag it as
secure.  The kernel driver will use this flag to determine
whether a rendering context is secure or not so that the
engine can be transitioned between secure or unsecure
or the work can be submitted to a secure queue depending
on the IP.

v2: the flag will be used for security, so remove the comment (Ray)

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index f90b453..7aab4e1 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -207,6 +207,9 @@ union drm_amdgpu_bo_list {
 #define AMDGPU_CTX_OP_QUERY_STATE	3
 #define AMDGPU_CTX_OP_QUERY_STATE2	4
 
+/* Flag the context as secure */
+#define AMDGPU_CTX_ALLOC_FLAGS_SECURE	(1 << 0)
+
 /* GPU reset status */
 #define AMDGPU_CTX_NO_RESET		0
 /* this the context caused it */
@@ -241,7 +244,6 @@ union drm_amdgpu_bo_list {
 struct drm_amdgpu_ctx_in {
 	/** AMDGPU_CTX_OP_* */
 	__u32	op;
-	/** For future use, no flags defined so far */
 	__u32	flags;
 	__u32	ctx_id;
 	/** AMDGPU_CTX_PRIORITY_* */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/14] drm/amdgpu: define the TMZ bit for the PTE

[Huang, Ray]

From: Alex Deucher <alexander.deucher@amd.com>

Define the TMZ (encryption) bit in the page table entry (PTE) for
Raven and newer asics.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 3352a87..4b5d283 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -53,6 +53,9 @@ struct amdgpu_bo_list_entry;
 #define AMDGPU_PTE_SYSTEM	(1ULL << 1)
 #define AMDGPU_PTE_SNOOPED	(1ULL << 2)
 
+/* RV+ */
+#define AMDGPU_PTE_TMZ		(1ULL << 3)
+
 /* VI only */
 #define AMDGPU_PTE_EXECUTABLE	(1ULL << 4)
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 04/14] drm/amdgpu: add tmz feature parameter (v2)

[Huang, Ray]

This patch adds tmz parameter to enable/disable the feature in the amdgpu kernel
module. Nomally, by default, it should be auto (rely on the hardware
capability).

But right now, it need to set "off" to avoid breaking other developers'
work because it's not totally completed.

Will set "auto" till the feature is stable and completely verified.

v2: add "auto" option for future use.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a1516a3..930643c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -172,6 +172,7 @@ extern int amdgpu_force_asic_type;
 #ifdef CONFIG_HSA_AMD
 extern int sched_policy;
 #endif
+extern int amdgpu_tmz;
 
 #ifdef CONFIG_DRM_AMDGPU_SI
 extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6978d17..606f1d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -145,6 +145,7 @@ int amdgpu_discovery = -1;
 int amdgpu_mes = 0;
 int amdgpu_noretry = 1;
 int amdgpu_force_asic_type = -1;
+int amdgpu_tmz = 0;
 
 struct amdgpu_mgpu_info mgpu_info = {
 	.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
@@ -752,6 +753,16 @@ uint amdgpu_dm_abm_level = 0;
 MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
 module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
 
+/**
+ * DOC: tmz (int)
+ * Trust Memory Zone (TMZ) is a method to protect the contents being written to
+ * and read from memory.
+ *
+ * The default value: 0 (off).  TODO: change to auto till it is completed.
+ */
+MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = on)");
+module_param_named(tmz, amdgpu_tmz, int, 0444);
+
 static const struct pci_device_id pciidlist[] = {
 #ifdef  CONFIG_DRM_AMDGPU_SI
 	{0x1002, 0x6780, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TAHITI},
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/14] drm/amdgpu: add amdgpu_tmz data structure

[Huang, Ray]

This patch to add amdgpu_tmz structure which stores all tmz related fields.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  6 +++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h | 36 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 930643c..697e8e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -89,6 +89,7 @@
 #include "amdgpu_mes.h"
 #include "amdgpu_umc.h"
 #include "amdgpu_mmhub.h"
+#include "amdgpu_tmz.h"
 
 #define MAX_GPU_INSTANCE		16
 
@@ -916,6 +917,9 @@ struct amdgpu_device {
 	bool                            enable_mes;
 	struct amdgpu_mes               mes;
 
+	/* tmz */
+	struct amdgpu_tmz		tmz;
+
 	struct amdgpu_ip_block          ip_blocks[AMDGPU_MAX_IP_NUM];
 	int				num_ip_blocks;
 	struct mutex	mn_lock;
@@ -927,7 +931,7 @@ struct amdgpu_device {
 	atomic64_t gart_pin_size;
 
 	/* soc15 register offset based on ip, instance and  segment */
-	uint32_t 		*reg_offset[MAX_HWIP][HWIP_MAX_INSTANCE];
+	uint32_t		*reg_offset[MAX_HWIP][HWIP_MAX_INSTANCE];
 
 	const struct amdgpu_df_funcs	*df_funcs;
 	const struct amdgpu_mmhub_funcs	*mmhub_funcs;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
new file mode 100644
index 0000000..24bbbc2
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2019 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_TMZ_H__
+#define __AMDGPU_TMZ_H__
+
+#include "amdgpu.h"
+
+/*
+ * Trust memory zone stuff
+ */
+struct amdgpu_tmz {
+	bool	enabled;
+};
+
+#endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/14] drm/amdgpu: add function to check tmz capability (v4)

[Huang, Ray]

Add a function to check tmz capability with kernel parameter and ASIC type.

v2: use a per device tmz variable instead of global amdgpu_tmz.
v3: refine the comments for the function. (Luben)
v4: add amdgpu_tmz.c/h for future use.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c    | 49 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h    |  3 ++
 4 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 91369c8..270ce82 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -55,7 +55,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
 	amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
 	amdgpu_vm_sdma.o amdgpu_pmu.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
-	amdgpu_umc.o smu_v11_0_i2c.o
+	amdgpu_umc.o smu_v11_0_i2c.o amdgpu_tmz.o
 
 amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2535db2..e376fe5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -63,6 +63,7 @@
 #include "amdgpu_xgmi.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_pmu.h"
+#include "amdgpu_tmz.h"
 
 #include <linux/suspend.h>
 
@@ -1032,6 +1033,8 @@ static int amdgpu_device_check_arguments(struct amdgpu_device *adev)
 
 	adev->firmware.load_type = amdgpu_ucode_get_load_type(adev, amdgpu_fw_load_type);
 
+	adev->tmz.enabled = amdgpu_is_tmz(adev);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
new file mode 100644
index 0000000..14a5500
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2019 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <drm/drmP.h>
+#include "amdgpu.h"
+#include "amdgpu_tmz.h"
+
+
+/**
+ * amdgpu_is_tmz - validate trust memory zone
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Return true if @dev supports trusted memory zones (TMZ), and return false if
+ * @dev does not support TMZ.
+ */
+bool amdgpu_is_tmz(struct amdgpu_device *adev)
+{
+	if (!amdgpu_tmz)
+		return false;
+
+	if (adev->asic_type < CHIP_RAVEN || adev->asic_type == CHIP_ARCTURUS) {
+		dev_warn(adev->dev, "doesn't support trusted memory zones (TMZ)\n");
+		return false;
+	}
+
+	dev_info(adev->dev, "TMZ feature is enabled\n");
+
+	return true;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
index 24bbbc2..28e0517 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
@@ -33,4 +33,7 @@ struct amdgpu_tmz {
 	bool	enabled;
 };
 
+
+extern bool amdgpu_is_tmz(struct amdgpu_device *adev);
+
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/14] drm/ttm: add helper to get buffer object with ttm_mem_reg

[Huang, Ray]

This patch is to add a helper to get corresponding buffer object with a pointer
to a struct ttm_mem_reg.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/drm/ttm/ttm_bo_driver.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index d69121c..264e6c3 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -786,6 +786,19 @@ static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
 	reservation_object_unlock(bo->resv);
 }
 
+/**
+ * ttm_mem_reg_to_bo
+ *
+ * @mem: A pointer to a struct ttm_mem_reg.
+ *
+ * Returns corresponding buffer object of the @mem.
+ */
+static inline
+struct ttm_buffer_object *ttm_mem_reg_to_bo(struct ttm_mem_reg *mem)
+{
+	return container_of(mem, struct ttm_buffer_object, mem);
+}
+
 /*
  * ttm_bo_util.c
  */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/14] drm/amdgpu: revise the function to allocate secure context (v2)

[Huang, Ray]

The is_secure flag will indicate the current conext is protected or not.

v2: while user mode asks to create a context, but if tmz is disabled, it should
return failure.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 19 +++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  1 +
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 45a30aa..ae28aec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -72,7 +72,8 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
 static int amdgpu_ctx_init(struct amdgpu_device *adev,
 			   enum drm_sched_priority priority,
 			   struct drm_file *filp,
-			   struct amdgpu_ctx *ctx)
+			   struct amdgpu_ctx *ctx,
+			   uint32_t flags)
 {
 	unsigned num_entities = amdgpu_ctx_total_num_entities();
 	unsigned i, j, k;
@@ -121,6 +122,9 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 	ctx->init_priority = priority;
 	ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
 
+	if (flags & AMDGPU_CTX_ALLOC_FLAGS_SECURE)
+		ctx->is_secure = true;
+
 	for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
 		struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
 		struct drm_sched_rq *rqs[AMDGPU_MAX_RINGS];
@@ -253,7 +257,7 @@ static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
 			    struct amdgpu_fpriv *fpriv,
 			    struct drm_file *filp,
 			    enum drm_sched_priority priority,
-			    uint32_t *id)
+			    uint32_t *id, uint32_t flags)
 {
 	struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
 	struct amdgpu_ctx *ctx;
@@ -272,7 +276,7 @@ static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
 	}
 
 	*id = (uint32_t)r;
-	r = amdgpu_ctx_init(adev, priority, filp, ctx);
+	r = amdgpu_ctx_init(adev, priority, filp, ctx, flags);
 	if (r) {
 		idr_remove(&mgr->ctx_handles, *id);
 		*id = 0;
@@ -407,6 +411,12 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 	struct amdgpu_device *adev = dev->dev_private;
 	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 
+	if (!adev->tmz.enabled &&
+	    (args->in.flags & AMDGPU_CTX_ALLOC_FLAGS_SECURE)) {
+		DRM_ERROR("Cannot allocate secure context while tmz is disabled\n");
+		return -EINVAL;
+	}
+
 	r = 0;
 	id = args->in.ctx_id;
 	priority = amdgpu_to_sched_priority(args->in.priority);
@@ -418,7 +428,8 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 
 	switch (args->in.op) {
 	case AMDGPU_CTX_OP_ALLOC_CTX:
-		r = amdgpu_ctx_alloc(adev, fpriv, filp, priority, &id);
+		r = amdgpu_ctx_alloc(adev, fpriv, filp, priority,
+				     &id, args->in.flags);
 		args->out.alloc.ctx_id = id;
 		break;
 	case AMDGPU_CTX_OP_FREE_CTX:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index da80863..aa8642b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -45,6 +45,7 @@ struct amdgpu_ctx {
 	struct dma_fence		**fences;
 	struct amdgpu_ctx_entity	*entities[AMDGPU_HW_IP_NUM];
 	bool				preamble_presented;
+	bool				is_secure;
 	enum drm_sched_priority		init_priority;
 	enum drm_sched_priority		override_priority;
 	struct mutex			lock;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/14] drm/amdgpu: add tmz bit in frame control packet

[Huang, Ray]

This patch adds tmz bit in frame control pm4 packet, and it will used in future.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/nvd.h    | 1 +
 drivers/gpu/drm/amd/amdgpu/soc15d.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nvd.h b/drivers/gpu/drm/amd/amdgpu/nvd.h
index 1de9846..f3d8771 100644
--- a/drivers/gpu/drm/amd/amdgpu/nvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/nvd.h
@@ -306,6 +306,7 @@
 #define	PACKET3_GET_LOD_STATS				0x8E
 #define	PACKET3_DRAW_MULTI_PREAMBLE			0x8F
 #define	PACKET3_FRAME_CONTROL				0x90
+#			define FRAME_TMZ	(1 << 0)
 #			define FRAME_CMD(x) ((x) << 28)
 			/*
 			 * x=0: tmz_begin
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15d.h b/drivers/gpu/drm/amd/amdgpu/soc15d.h
index edfe508..295d68c 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15d.h
+++ b/drivers/gpu/drm/amd/amdgpu/soc15d.h
@@ -286,6 +286,7 @@
 #define	PACKET3_WAIT_ON_DE_COUNTER_DIFF			0x88
 #define	PACKET3_SWITCH_BUFFER				0x8B
 #define PACKET3_FRAME_CONTROL				0x90
+#			define FRAME_TMZ	(1 << 0)
 #			define FRAME_CMD(x) ((x) << 28)
 			/*
 			 * x=0: tmz_begin
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/14] drm/amdgpu: expand the emit tmz interface with trusted flag

[Huang, Ray]

This patch expands the emit_tmz function to support trusted flag while we want
to set command buffer in trusted mode.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 ++--
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 16 ++++++++++++----
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 13 ++++++++++---
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 6882eeb..54741ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -229,7 +229,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	}
 
 	if (ring->funcs->emit_tmz)
-		amdgpu_ring_emit_tmz(ring, false);
+		amdgpu_ring_emit_tmz(ring, false, false);
 
 #ifdef CONFIG_X86_64
 	if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 930316e..34aa63a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -166,7 +166,7 @@ struct amdgpu_ring_funcs {
 	void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
 					uint32_t reg0, uint32_t reg1,
 					uint32_t ref, uint32_t mask);
-	void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
+	void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
 	/* priority functions */
 	void (*set_priority) (struct amdgpu_ring *ring,
 			      enum drm_sched_priority priority);
@@ -247,7 +247,7 @@ struct amdgpu_ring {
 #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
 #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
 #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
-#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
+#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
 #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
 #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
 #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index a2f4ff1..18f741b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -243,7 +243,8 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
 static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
 static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
 static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
-static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
+static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
+				    bool trusted);
 
 static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
 {
@@ -4521,7 +4522,7 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flag
 		gfx_v10_0_ring_emit_ce_meta(ring,
 				    flags & AMDGPU_IB_PREEMPTED ? true : false);
 
-	gfx_v10_0_ring_emit_tmz(ring, true);
+	gfx_v10_0_ring_emit_tmz(ring, true, false);
 
 	dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
 	if (flags & AMDGPU_HAVE_CTX_SWITCH) {
@@ -4679,10 +4680,17 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
 					   sizeof(de_payload) >> 2);
 }
 
-static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
+static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
+				    bool trusted)
 {
 	amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
-	amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */
+	/*
+	 * cmd = 0: frame begin
+	 * cmd = 1: frame end
+	 */
+	amdgpu_ring_write(ring,
+			  ((ring->adev->tmz.enabled && trusted) ? FRAME_TMZ : 0)
+			  | FRAME_CMD(start ? 0 : 1));
 }
 
 static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 90348fb29..fa264d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5240,10 +5240,17 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
 	amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
 }
 
-static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
+static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
+				   bool trusted)
 {
 	amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
-	amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */
+	/*
+	 * cmd = 0: frame begin
+	 * cmd = 1: frame end
+	 */
+	amdgpu_ring_write(ring,
+			  ((ring->adev->tmz.enabled && trusted) ? FRAME_TMZ : 0)
+			  | FRAME_CMD(start ? 0 : 1));
 }
 
 static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
@@ -5253,7 +5260,7 @@ static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
 	if (amdgpu_sriov_vf(ring->adev))
 		gfx_v9_0_ring_emit_ce_meta(ring);
 
-	gfx_v9_0_ring_emit_tmz(ring, true);
+	gfx_v9_0_ring_emit_tmz(ring, true, false);
 
 	dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
 	if (flags & AMDGPU_HAVE_CTX_SWITCH) {
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 11/14] drm/amdgpu: expand the context control interface with trust flag

[Huang, Ray]

This patch expands the context control function to support trusted flag while we
want to set command buffer in trusted mode.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 +++--
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    | 3 ++-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    | 3 ++-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    | 3 ++-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 5 +++--
 7 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 54741ba..e1dc229 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -210,7 +210,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	if (job && ring->funcs->emit_cntxcntl) {
 		status |= job->preamble_status;
 		status |= job->preemption_status;
-		amdgpu_ring_emit_cntxcntl(ring, status);
+		amdgpu_ring_emit_cntxcntl(ring, status, false);
 	}
 
 	for (i = 0; i < num_ibs; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 34aa63a..5134d0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -158,7 +158,8 @@ struct amdgpu_ring_funcs {
 	void (*begin_use)(struct amdgpu_ring *ring);
 	void (*end_use)(struct amdgpu_ring *ring);
 	void (*emit_switch_buffer) (struct amdgpu_ring *ring);
-	void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
+	void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
+			       bool trusted);
 	void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
 	void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
 	void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
@@ -242,7 +243,7 @@ struct amdgpu_ring {
 #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
 #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
 #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
-#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
+#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
 #define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
 #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
 #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 18f741b..06698c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4514,7 +4514,9 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, 0);
 }
 
-static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
+					 uint32_t flags,
+					 bool trusted)
 {
 	uint32_t dw2 = 0;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 8c27c30..b4af1b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -2972,7 +2972,8 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
 	return clock;
 }
 
-static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
+				      bool trusted)
 {
 	if (flags & AMDGPU_HAVE_CTX_SWITCH)
 		gfx_v6_0_ring_emit_vgt_flush(ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 48796b68..c08f5c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2309,7 +2309,8 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 	amdgpu_ring_write(ring, control);
 }
 
-static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
+				      bool trusted)
 {
 	uint32_t dw2 = 0;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 98e5aa8..d3a23fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6393,7 +6393,8 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, 0);
 }
 
-static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
+				      bool trusted)
 {
 	uint32_t dw2 = 0;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index fa264d5..872d100 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5253,14 +5253,15 @@ static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
 			  | FRAME_CMD(start ? 0 : 1));
 }
 
-static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
+				      bool trusted)
 {
 	uint32_t dw2 = 0;
 
 	if (amdgpu_sriov_vf(ring->adev))
 		gfx_v9_0_ring_emit_ce_meta(ring);
 
-	gfx_v9_0_ring_emit_tmz(ring, true, false);
+	gfx_v9_0_ring_emit_tmz(ring, true, trusted);
 
 	dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
 	if (flags & AMDGPU_HAVE_CTX_SWITCH) {
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 12/14] drm/amdgpu: set trusted mode while the job is under secure context (v2)

[Huang, Ray]

While user mode submit a command with secure context, we should set the command
buffer with trusted mode.

v2: fix the null job pointer while in vmid 0 submission.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 51f3db0..60e7b79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1252,6 +1252,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 		p->ctx->preamble_presented = true;
 	}
 
+	job->secure = p->ctx->is_secure;
 	cs->out.handle = seq;
 	job->uf_sequence = seq;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index e1dc229..cb9b650 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -210,7 +210,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	if (job && ring->funcs->emit_cntxcntl) {
 		status |= job->preamble_status;
 		status |= job->preemption_status;
-		amdgpu_ring_emit_cntxcntl(ring, status, false);
+		amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
 	}
 
 	for (i = 0; i < num_ibs; ++i) {
@@ -229,7 +229,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	}
 
 	if (ring->funcs->emit_tmz)
-		amdgpu_ring_emit_tmz(ring, false, false);
+		amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
 
 #ifdef CONFIG_X86_64
 	if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index dc7ee93..59f1dbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -63,6 +63,8 @@ struct amdgpu_job {
 	uint64_t		uf_addr;
 	uint64_t		uf_sequence;
 
+	/* the job is under secure context */
+	bool			secure;
 };
 
 int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 13/14] drm/amdgpu: modify the method to use mem under buffer object for amdgpu_ttm_tt_pte_flags

[Huang, Ray]

amdgpu_ttm_tt_pte_flags will be used for updating tmz bits while the bo is
secure, so we need pass the ttm_mem_reg under a buffer object.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c05638c..3663655 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1117,8 +1117,8 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	} else {
 
 		/* allocate GART space */
-		tmp = bo->mem;
-		tmp.mm_node = NULL;
+		tmp = bo->mem; /* cache bo->mem */
+		bo->mem.mm_node = NULL;
 		placement.num_placement = 1;
 		placement.placement = &placements;
 		placement.num_busy_placement = 1;
@@ -1128,23 +1128,25 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 		placements.flags = (bo->mem.placement & ~TTM_PL_MASK_MEM) |
 			TTM_PL_FLAG_TT;
 
-		r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
-		if (unlikely(r))
+		r = ttm_bo_mem_space(bo, &placement, &bo->mem, &ctx);
+		if (unlikely(r)) {
+			bo->mem = tmp;
 			return r;
+		}
 
 		/* compute PTE flags for this buffer object */
-		flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
+		flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &bo->mem);
 
 		/* Bind pages */
-		gtt->offset = (u64)tmp.start << PAGE_SHIFT;
+		gtt->offset = (u64)bo->mem.start << PAGE_SHIFT;
 		r = amdgpu_ttm_gart_bind(adev, bo, flags);
 		if (unlikely(r)) {
+			bo->mem = tmp;
 			ttm_bo_mem_put(bo, &tmp);
 			return r;
 		}
 
-		ttm_bo_mem_put(bo, &bo->mem);
-		bo->mem = tmp;
+		ttm_bo_mem_put(bo, &tmp);
 	}
 
 	bo->offset = (bo->mem.start << PAGE_SHIFT) +
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Huang, Ray]

From: Alex Deucher <alexander.deucher@amd.com>

If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED), the TMZ bits of
PTEs that belongs that bo should be set. Then psp is able to protect the pages
of this bo to avoid the access from an "untrust" domain such as CPU.

v1: design and draft the skeletion of tmz bits setting on PTEs (Alex)
v2: return failure once create secure bo on no-tmz platform  (Ray)

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 22eab74..5332104 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
 		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
 		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
-		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
+		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
+		      AMDGPU_GEM_CREATE_ENCRYPTED))
 
 		return -EINVAL;
 
@@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
 		return -EINVAL;
 
+	if (!adev->tmz.enabled && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
+		DRM_ERROR("Cannot allocate secure buffer while tmz is disabled\n");
+		return -EINVAL;
+	}
+
 	/* create a gem object to contain this object in */
 	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
 	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
@@ -251,6 +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 		resv = vm->root.base.bo->tbo.resv;
 	}
 
+	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
+		/* XXX: pad out alignment to meet TMZ requirements */
+	}
+
 	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
 				     (u32)(0xffffffff & args->in.domains),
 				     flags, ttm_bo_type_device, resv, &gobj);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 5a3c177..286e2e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -224,6 +224,16 @@ static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
 	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
 }
 
+/**
+ * amdgpu_bo_encrypted - return whether the bo is encrypted
+ */
+static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+	return adev->tmz.enabled && (bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED);
+}
+
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
 void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3663655..8f00bb2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
 uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
 {
 	uint64_t flags = 0;
+	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
+	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
 
 	if (mem && mem->mem_type != TTM_PL_SYSTEM)
 		flags |= AMDGPU_PTE_VALID;
@@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
 		if (ttm->caching_state == tt_cached)
 			flags |= AMDGPU_PTE_SNOOPED;
 	}
+	if (amdgpu_bo_encrypted(abo)) {
+		flags |= AMDGPU_PTE_TMZ;
+	}
 
 	return flags;
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/14] drm/amdgpu: introduce secure buffer object support (trusted memory zone)

[Koenig, Christian]

Patches #1-#4, #8, #9 are Reviewed-by: Christian König 
<christian.koenig@amd.com>

Patches #10, #11 are Acked-by: Christian König <christian.koenig@amd.com>

Patches #7 and the resulting workaround in patch #13 are a clear NAK. 
The ttm_mem_reg can't be used like this to get back to the ttm_bo object.

Going to reply separately on patch #14 regarding this.

Regards,
Christian.

Am 11.09.19 um 13:50 schrieb Huang, Ray:
> Hi all,
>
> These series of patches introduce a feature to support secure buffer object.
> The Trusted Memory Zone (TMZ) is a method to protect the contents being written
> to and read from memory. We use TMZ hardware memory protection scheme to
> implement the secure buffer object support.
>
> TMZ is the page-level protection that hardware will detect the TMZ bit in the
> page table entry to set the current page is encrypted. With this hardware
> feature, we design a BO-level protection in kernel driver to provide a new flag
> AMDGPU_GEM_CREATE_ENCRYPTED to gem create ioctl to libdrm for the secure buffer
> allocation. And also provide the AMDGPU_CTX_ALLOC_FLAGS_SECURE to indicate the
> context is trusted or not. If the BO is secure, then the data is encrypted, only
> the trusted IP blocks such as gfx, sdma, vcn are able to decrypt. CPU as the
> un-trusted IP are unable to read the secure buffer.
>
> We will submit the new secure context interface later for libdrm, and create a
> new test suite to verify the security feature in the libdrm unit tests.
>
> Suite id = 11: Name 'Security Tests status: ENABLED'
> Test id 1: Name: 'allocate secure buffer test status: ENABLED'
> Test id 2: Name: 'graphics command submission under secure context status: ENABLED'
>
> Thanks,
> Ray
>
> Alex Deucher (4):
>    drm/amdgpu: add UAPI for creating encrypted buffers
>    drm/amdgpu: add UAPI for creating secure contexts (v2)
>    drm/amdgpu: define the TMZ bit for the PTE
>    drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)
>
> Huang Rui (10):
>    drm/amdgpu: add tmz feature parameter (v2)
>    drm/amdgpu: add amdgpu_tmz data structure
>    drm/amdgpu: add function to check tmz capability (v4)
>    drm/ttm: add helper to get buffer object with ttm_mem_reg
>    drm/amdgpu: revise the function to allocate secure context (v2)
>    drm/amdgpu: add tmz bit in frame control packet
>    drm/amdgpu: expand the emit tmz interface with trusted flag
>    drm/amdgpu: expand the context control interface with trust flag
>    drm/amdgpu: set trusted mode while the job is under secure context
>      (v2)
>    drm/amdgpu: modify the method to use mem under buffer object for
>      amdgpu_ttm_tt_pte_flags
>
>   drivers/gpu/drm/amd/amdgpu/Makefile        |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  7 ++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c    | 19 +++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h    |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 11 +++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c     |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h    |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  9 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c    | 49 ++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h    | 39 ++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 23 +++++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  3 ++
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     | 20 +++++++++---
>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c      |  3 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c      |  3 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  3 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 16 +++++++---
>   drivers/gpu/drm/amd/amdgpu/nvd.h           |  1 +
>   drivers/gpu/drm/amd/amdgpu/soc15d.h        |  1 +
>   include/drm/ttm/ttm_bo_driver.h            | 13 ++++++++
>   include/uapi/drm/amdgpu_drm.h              |  9 +++++-
>   25 files changed, 230 insertions(+), 34 deletions(-)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Koenig, Christian]

Am 11.09.19 um 13:50 schrieb Huang, Ray:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED), the TMZ bits of
> PTEs that belongs that bo should be set. Then psp is able to protect the pages
> of this bo to avoid the access from an "untrust" domain such as CPU.
>
> v1: design and draft the skeletion of tmz bits setting on PTEs (Alex)
> v2: return failure once create secure bo on no-tmz platform  (Ray)
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
>   3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 22eab74..5332104 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>   		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
>   		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
> +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> +		      AMDGPU_GEM_CREATE_ENCRYPTED))
>   
>   		return -EINVAL;
>   
> @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>   		return -EINVAL;
>   
> +	if (!adev->tmz.enabled && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
> +		DRM_ERROR("Cannot allocate secure buffer while tmz is disabled\n");
> +		return -EINVAL;
> +	}
> +
>   	/* create a gem object to contain this object in */
>   	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>   	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
> @@ -251,6 +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   		resv = vm->root.base.bo->tbo.resv;
>   	}
>   
> +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
> +		/* XXX: pad out alignment to meet TMZ requirements */
> +	}
> +
>   	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
>   				     (u32)(0xffffffff & args->in.domains),
>   				     flags, ttm_bo_type_device, resv, &gobj);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 5a3c177..286e2e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -224,6 +224,16 @@ static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
>   	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
>   }
>   
> +/**
> + * amdgpu_bo_encrypted - return whether the bo is encrypted
> + */
> +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +
> +	return adev->tmz.enabled && (bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED);

Checking the adev->tmz.enabled flags should be dropped here.

> +}
> +
>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>   void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 3663655..8f00bb2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
>   uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>   {
>   	uint64_t flags = 0;
> +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
> +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);

That's a clear NAK. The function is not necessarily called with 
&bo->mem, which is also the reason why this function doesn't gets the BO 
as parameter.

Christian.

>   
>   	if (mem && mem->mem_type != TTM_PL_SYSTEM)
>   		flags |= AMDGPU_PTE_VALID;
> @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>   		if (ttm->caching_state == tt_cached)
>   			flags |= AMDGPU_PTE_SNOOPED;
>   	}
> +	if (amdgpu_bo_encrypted(abo)) {
> +		flags |= AMDGPU_PTE_TMZ;
> +	}
>   
>   	return flags;
>   }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Huang, Ray]

On Wed, Sep 11, 2019 at 08:13:19PM +0800, Koenig, Christian wrote:
> Am 11.09.19 um 13:50 schrieb Huang, Ray:
> > From: Alex Deucher <alexander.deucher@amd.com>
> >
> > If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED), the TMZ bits of
> > PTEs that belongs that bo should be set. Then psp is able to protect the pages
> > of this bo to avoid the access from an "untrust" domain such as CPU.
> >
> > v1: design and draft the skeletion of tmz bits setting on PTEs (Alex)
> > v2: return failure once create secure bo on no-tmz platform  (Ray)
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > Reviewed-by: Huang Rui <ray.huang@amd.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
> >   3 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > index 22eab74..5332104 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
> >   		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> >   		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
> >   		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> > -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
> > +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> > +		      AMDGPU_GEM_CREATE_ENCRYPTED))
> >   
> >   		return -EINVAL;
> >   
> > @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
> >   	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
> >   		return -EINVAL;
> >   
> > +	if (!adev->tmz.enabled && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
> > +		DRM_ERROR("Cannot allocate secure buffer while tmz is disabled\n");
> > +		return -EINVAL;
> > +	}
> > +
> >   	/* create a gem object to contain this object in */
> >   	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
> >   	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
> > @@ -251,6 +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
> >   		resv = vm->root.base.bo->tbo.resv;
> >   	}
> >   
> > +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
> > +		/* XXX: pad out alignment to meet TMZ requirements */
> > +	}
> > +
> >   	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
> >   				     (u32)(0xffffffff & args->in.domains),
> >   				     flags, ttm_bo_type_device, resv, &gobj);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index 5a3c177..286e2e2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -224,6 +224,16 @@ static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
> >   	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
> >   }
> >   
> > +/**
> > + * amdgpu_bo_encrypted - return whether the bo is encrypted
> > + */
> > +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
> > +{
> > +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> > +
> > +	return adev->tmz.enabled && (bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED);
> 
> Checking the adev->tmz.enabled flags should be dropped here.
> 

That's fine. BO should be validated while it is created.

But if the BO is created by vmid 0, is this checking needed?

> > +}
> > +
> >   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
> >   void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
> >   
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 3663655..8f00bb2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
> >   uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
> >   {
> >   	uint64_t flags = 0;
> > +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
> > +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
> 
> That's a clear NAK. The function is not necessarily called with 
> &bo->mem, which is also the reason why this function doesn't gets the BO 
> as parameter.
> 

Do you mean we can revise the below functions to use bo as the parameter
instead? 

uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct amdgpu_bo *bo)
uint64_t amdgpu_ttm_tt_pte_flags(struct ttm_tt *ttm, struct amdgpu_bo *bo)

Thanks,
Ray

> Christian.
> 
> >   
> >   	if (mem && mem->mem_type != TTM_PL_SYSTEM)
> >   		flags |= AMDGPU_PTE_VALID;
> > @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
> >   		if (ttm->caching_state == tt_cached)
> >   			flags |= AMDGPU_PTE_SNOOPED;
> >   	}
> > +	if (amdgpu_bo_encrypted(abo)) {
> > +		flags |= AMDGPU_PTE_TMZ;
> > +	}
> >   
> >   	return flags;
> >   }
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Koenig, Christian]

Am 12.09.19 um 12:27 schrieb Huang, Ray:
> On Wed, Sep 11, 2019 at 08:13:19PM +0800, Koenig, Christian wrote:
>> Am 11.09.19 um 13:50 schrieb Huang, Ray:
>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>
>>> If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED), the TMZ bits of
>>> PTEs that belongs that bo should be set. Then psp is able to protect the pages
>>> of this bo to avoid the access from an "untrust" domain such as CPU.
>>>
>>> v1: design and draft the skeletion of tmz bits setting on PTEs (Alex)
>>> v2: return failure once create secure bo on no-tmz platform  (Ray)
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Reviewed-by: Huang Rui <ray.huang@amd.com>
>>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
>>>    3 files changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 22eab74..5332104 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>>>    		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>>>    		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
>>>    		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
>>> -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
>>> +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
>>> +		      AMDGPU_GEM_CREATE_ENCRYPTED))
>>>    
>>>    		return -EINVAL;
>>>    
>>> @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>>>    	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>>>    		return -EINVAL;
>>>    
>>> +	if (!adev->tmz.enabled && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
>>> +		DRM_ERROR("Cannot allocate secure buffer while tmz is disabled\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>>    	/* create a gem object to contain this object in */
>>>    	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>>>    	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
>>> @@ -251,6 +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>>>    		resv = vm->root.base.bo->tbo.resv;
>>>    	}
>>>    
>>> +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
>>> +		/* XXX: pad out alignment to meet TMZ requirements */
>>> +	}
>>> +
>>>    	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
>>>    				     (u32)(0xffffffff & args->in.domains),
>>>    				     flags, ttm_bo_type_device, resv, &gobj);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 5a3c177..286e2e2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -224,6 +224,16 @@ static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
>>>    	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
>>>    }
>>>    
>>> +/**
>>> + * amdgpu_bo_encrypted - return whether the bo is encrypted
>>> + */
>>> +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
>>> +{
>>> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> +
>>> +	return adev->tmz.enabled && (bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED);
>> Checking the adev->tmz.enabled flags should be dropped here.
>>
> That's fine. BO should be validated while it is created.
>
> But if the BO is created by vmid 0, is this checking needed?
>
>>> +}
>>> +
>>>    bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>>>    void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
>>>    
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 3663655..8f00bb2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
>>>    uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>>>    {
>>>    	uint64_t flags = 0;
>>> +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
>>> +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
>> That's a clear NAK. The function is not necessarily called with
>> &bo->mem, which is also the reason why this function doesn't gets the BO
>> as parameter.
>>
> Do you mean we can revise the below functions to use bo as the parameter
> instead?
>
> uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct amdgpu_bo *bo)
> uint64_t amdgpu_ttm_tt_pte_flags(struct ttm_tt *ttm, struct amdgpu_bo *bo)

If that is possible then this would indeed be a solution for the problem.

Christian.

>
> Thanks,
> Ray
>
>> Christian.
>>
>>>    
>>>    	if (mem && mem->mem_type != TTM_PL_SYSTEM)
>>>    		flags |= AMDGPU_PTE_VALID;
>>> @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>>>    		if (ttm->caching_state == tt_cached)
>>>    			flags |= AMDGPU_PTE_SNOOPED;
>>>    	}
>>> +	if (amdgpu_bo_encrypted(abo)) {
>>> +		flags |= AMDGPU_PTE_TMZ;
>>> +	}
>>>    
>>>    	return flags;
>>>    }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Huang, Ray]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Thursday, September 12, 2019 7:49 PM
> To: Huang, Ray <Ray.Huang@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> Deucher, Alexander <Alexander.Deucher@amd.com>; Tuikov, Luben
> <Luben.Tuikov@amd.com>; Liu, Aaron <Aaron.Liu@amd.com>
> Subject: Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo
> (v2)
> 
> Am 12.09.19 um 12:27 schrieb Huang, Ray:
> > On Wed, Sep 11, 2019 at 08:13:19PM +0800, Koenig, Christian wrote:
> >> Am 11.09.19 um 13:50 schrieb Huang, Ray:
> >>> From: Alex Deucher <alexander.deucher@amd.com>
> >>>
> >>> If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED),
> the
> >>> TMZ bits of PTEs that belongs that bo should be set. Then psp is
> >>> able to protect the pages of this bo to avoid the access from an "untrust"
> domain such as CPU.
> >>>
> >>> v1: design and draft the skeletion of tmz bits setting on PTEs
> >>> (Alex)
> >>> v2: return failure once create secure bo on no-tmz platform  (Ray)
> >>>
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> Reviewed-by: Huang Rui <ray.huang@amd.com>
> >>> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >>> ---
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
> >>>    3 files changed, 26 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> index 22eab74..5332104 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device
> *dev, void *data,
> >>>    		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> >>>    		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
> >>>    		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> >>> -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
> >>> +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> >>> +		      AMDGPU_GEM_CREATE_ENCRYPTED))
> >>>
> >>>    		return -EINVAL;
> >>>
> >>> @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct
> drm_device *dev, void *data,
> >>>    	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
> >>>    		return -EINVAL;
> >>>
> >>> +	if (!adev->tmz.enabled && (flags &
> AMDGPU_GEM_CREATE_ENCRYPTED)) {
> >>> +		DRM_ERROR("Cannot allocate secure buffer while tmz is
> disabled\n");
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>>    	/* create a gem object to contain this object in */
> >>>    	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
> >>>    	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA))
> { @@ -251,6
> >>> +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev,
> void *data,
> >>>    		resv = vm->root.base.bo->tbo.resv;
> >>>    	}
> >>>
> >>> +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
> >>> +		/* XXX: pad out alignment to meet TMZ requirements */
> >>> +	}
> >>> +
> >>>    	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
> >>>    				     (u32)(0xffffffff & args->in.domains),
> >>>    				     flags, ttm_bo_type_device, resv, &gobj);
> diff --git
> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >>> index 5a3c177..286e2e2 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >>> @@ -224,6 +224,16 @@ static inline bool
> amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
> >>>    	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
> >>>    }
> >>>
> >>> +/**
> >>> + * amdgpu_bo_encrypted - return whether the bo is encrypted  */
> >>> +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo) {
> >>> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> >>> +
> >>> +	return adev->tmz.enabled && (bo->flags &
> >>> +AMDGPU_GEM_CREATE_ENCRYPTED);
> >> Checking the adev->tmz.enabled flags should be dropped here.
> >>
> > That's fine. BO should be validated while it is created.
> >
> > But if the BO is created by vmid 0, is this checking needed?
> >
> >>> +}
> >>> +
> >>>    bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
> >>>    void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo,
> u32
> >>> domain);
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> index 3663655..8f00bb2 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct
> ttm_tt *ttm)
> >>>    uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct
> ttm_mem_reg *mem)
> >>>    {
> >>>    	uint64_t flags = 0;
> >>> +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
> >>> +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
> >> That's a clear NAK. The function is not necessarily called with
> >> &bo->mem, which is also the reason why this function doesn't gets the
> >> BO as parameter.
> >>
> > Do you mean we can revise the below functions to use bo as the
> > parameter instead?
> >
> > uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct amdgpu_bo
> > *bo) uint64_t amdgpu_ttm_tt_pte_flags(struct ttm_tt *ttm, struct
> > amdgpu_bo *bo)
> 
> If that is possible then this would indeed be a solution for the problem.
> 

Sorry to late response, I was revising the unit test for secure memory few days ago. 

Most of cases can be updated to amdgpu_ttm_tt_pte_flags with amdgpu_bo as the parameter except one case in
amdgpu_ttm_backend_bind(). It will be called by ttm_tt_bind() under amdgpu_move_vram_ram(). But ttm_mem_reg is new assigned. 

How about using modify the bind callback in ttm_backend_func:

int (*bind) (struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem);

Then we can update the following functions as:

uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_buffer_object *tbo,  struct ttm_mem_reg *mem);
uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_buffer_object *tbo, struct ttm_mem_reg *mem);

It looks better than before.

Thanks,
Ray

> Christian.
> 
> >
> > Thanks,
> > Ray
> >
> >> Christian.
> >>
> >>>
> >>>    	if (mem && mem->mem_type != TTM_PL_SYSTEM)
> >>>    		flags |= AMDGPU_PTE_VALID;
> >>> @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
> ttm_tt *ttm, struct ttm_mem_reg *mem)
> >>>    		if (ttm->caching_state == tt_cached)
> >>>    			flags |= AMDGPU_PTE_SNOOPED;
> >>>    	}
> >>> +	if (amdgpu_bo_encrypted(abo)) {
> >>> +		flags |= AMDGPU_PTE_TMZ;
> >>> +	}
> >>>
> >>>    	return flags;
> >>>    }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Koenig, Christian]

Am 24.09.19 um 13:48 schrieb Huang, Ray:
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Thursday, September 12, 2019 7:49 PM
>> To: Huang, Ray <Ray.Huang@amd.com>
>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>> Deucher, Alexander <Alexander.Deucher@amd.com>; Tuikov, Luben
>> <Luben.Tuikov@amd.com>; Liu, Aaron <Aaron.Liu@amd.com>
>> Subject: Re: [PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo
>> (v2)
>>
>> Am 12.09.19 um 12:27 schrieb Huang, Ray:
>>> On Wed, Sep 11, 2019 at 08:13:19PM +0800, Koenig, Christian wrote:
>>>> Am 11.09.19 um 13:50 schrieb Huang, Ray:
>>>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>>>
>>>>> If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED),
>> the
>>>>> TMZ bits of PTEs that belongs that bo should be set. Then psp is
>>>>> able to protect the pages of this bo to avoid the access from an "untrust"
>> domain such as CPU.
>>>>> v1: design and draft the skeletion of tmz bits setting on PTEs
>>>>> (Alex)
>>>>> v2: return failure once create secure bo on no-tmz platform  (Ray)
>>>>>
>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>>> Reviewed-by: Huang Rui <ray.huang@amd.com>
>>>>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
>>>>>     3 files changed, 26 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> index 22eab74..5332104 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device
>> *dev, void *data,
>>>>>     		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>>>>>     		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
>>>>>     		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
>>>>> -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
>>>>> +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
>>>>> +		      AMDGPU_GEM_CREATE_ENCRYPTED))
>>>>>
>>>>>     		return -EINVAL;
>>>>>
>>>>> @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct
>> drm_device *dev, void *data,
>>>>>     	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>>>>>     		return -EINVAL;
>>>>>
>>>>> +	if (!adev->tmz.enabled && (flags &
>> AMDGPU_GEM_CREATE_ENCRYPTED)) {
>>>>> +		DRM_ERROR("Cannot allocate secure buffer while tmz is
>> disabled\n");
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>>     	/* create a gem object to contain this object in */
>>>>>     	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>>>>>     	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA))
>> { @@ -251,6
>>>>> +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev,
>> void *data,
>>>>>     		resv = vm->root.base.bo->tbo.resv;
>>>>>     	}
>>>>>
>>>>> +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
>>>>> +		/* XXX: pad out alignment to meet TMZ requirements */
>>>>> +	}
>>>>> +
>>>>>     	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
>>>>>     				     (u32)(0xffffffff & args->in.domains),
>>>>>     				     flags, ttm_bo_type_device, resv, &gobj);
>> diff --git
>>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> index 5a3c177..286e2e2 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>>> @@ -224,6 +224,16 @@ static inline bool
>> amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
>>>>>     	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
>>>>>     }
>>>>>
>>>>> +/**
>>>>> + * amdgpu_bo_encrypted - return whether the bo is encrypted  */
>>>>> +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo) {
>>>>> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>>> +
>>>>> +	return adev->tmz.enabled && (bo->flags &
>>>>> +AMDGPU_GEM_CREATE_ENCRYPTED);
>>>> Checking the adev->tmz.enabled flags should be dropped here.
>>>>
>>> That's fine. BO should be validated while it is created.
>>>
>>> But if the BO is created by vmid 0, is this checking needed?
>>>
>>>>> +}
>>>>> +
>>>>>     bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>>>>>     void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo,
>> u32
>>>>> domain);
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> index 3663655..8f00bb2 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct
>> ttm_tt *ttm)
>>>>>     uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct
>> ttm_mem_reg *mem)
>>>>>     {
>>>>>     	uint64_t flags = 0;
>>>>> +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
>>>>> +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
>>>> That's a clear NAK. The function is not necessarily called with
>>>> &bo->mem, which is also the reason why this function doesn't gets the
>>>> BO as parameter.
>>>>
>>> Do you mean we can revise the below functions to use bo as the
>>> parameter instead?
>>>
>>> uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct amdgpu_bo
>>> *bo) uint64_t amdgpu_ttm_tt_pte_flags(struct ttm_tt *ttm, struct
>>> amdgpu_bo *bo)
>> If that is possible then this would indeed be a solution for the problem.
>>
> Sorry to late response, I was revising the unit test for secure memory few days ago.
>
> Most of cases can be updated to amdgpu_ttm_tt_pte_flags with amdgpu_bo as the parameter except one case in
> amdgpu_ttm_backend_bind(). It will be called by ttm_tt_bind() under amdgpu_move_vram_ram(). But ttm_mem_reg is new assigned.
>
> How about using modify the bind callback in ttm_backend_func:
>
> int (*bind) (struct ttm_buffer_object *bo, struct ttm_mem_reg *bo_mem);

That won't work correctly.

Binding and unbinding the ttm_mem_reg from the GART is separate to the 
BO. E.g. the BO can long be destroyed or it could be a ghost BO.

>
> Then we can update the following functions as:
>
> uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_buffer_object *tbo,  struct ttm_mem_reg *mem);
> uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_buffer_object *tbo, struct ttm_mem_reg *mem);
>
> It looks better than before.

The whole approach of adding the TMZ flag to amdgpu_ttm_tt_pte_flags() 
is completely broken by design. Rather add adding the flag to 
amdgpu_vm_bo_update() instead.

Regards,
Christian.

>
> Thanks,
> Ray
>
>> Christian.
>>
>>> Thanks,
>>> Ray
>>>
>>>> Christian.
>>>>
>>>>>     	if (mem && mem->mem_type != TTM_PL_SYSTEM)
>>>>>     		flags |= AMDGPU_PTE_VALID;
>>>>> @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
>> ttm_tt *ttm, struct ttm_mem_reg *mem)
>>>>>     		if (ttm->caching_state == tt_cached)
>>>>>     			flags |= AMDGPU_PTE_SNOOPED;
>>>>>     	}
>>>>> +	if (amdgpu_bo_encrypted(abo)) {
>>>>> +		flags |= AMDGPU_PTE_TMZ;
>>>>> +	}
>>>>>
>>>>>     	return flags;
>>>>>     }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx