Re: [PATCH] drm/amdgpu: implement TMZ accessor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 01.11.19 um 19:24 schrieb Tuikov, Luben:
On 2019-11-01 14:14, Alex Deucher wrote:
On Thu, Oct 31, 2019 at 8:15 PM Tuikov, Luben <Luben.Tuikov@xxxxxxx> wrote:
On 2019-10-31 3:29 a.m., Christian König wrote:
Am 31.10.19 um 00:43 schrieb Tuikov, Luben:
Implement an accessor of adev->tmz.enabled. Let not
code around access it as "if (adev->tmz.enabled)"
as the organization may change. Instead...

Recruit "bool amdgpu_is_tmz(adev)" to return
exactly this Boolean value. That is, this function
is now an accessor of an already initialized and
set adev and adev->tmz.

Add "void amdgpu_tmz_set(adev)" to check and set
adev->tmz.* at initialization time. After which
one uses "bool amdgpu_is_tmz(adev)" to query
whether adev supports TMZ.
Actually I would rather completely remove the amdgpu_tmz.[hc] files. See
TMZ is a feature and not a hardware block.

All that stuff should probably moved into the PSP handling, since the
control of TMZ is enabled or disabled in the hardware is there.
Hi Christian,

Thanks for reviewing this. Sure, we can do that as well, should
there be consensus on it.

I just saw myself needing to know if the device is TMZ (as well
as if a buffer is TMZ for which I used amdgpu_bo_encrypted())
and so it became natural to write (after this patch):

if (!amdgpu_bo_encrypted(abo) || !amdgpu_is_tmz(adev)) {
         /* normal processing */
} else {
         /* TMZ processing */
}

BTW, should we proceed as you suggested, do you see
those primitives going into psp_v12_0.c?

They are not psp version specific.  the PSP handles the security, but
it's not really involved much from a driver perspective; they are
really more of a memory management thing.  I'd suggest putting them in
struct amdgpu_gmc.
Thanks Alex. So then I'd get rid of the files and consolidate
into struct amdgpu_gmc, with the files disappearing as Christian
suggested.

Sounds like a plan to me as well.

Thanks,
Christian.


Regards,
Luben

Alex

Regards,
Luben

Regards,
Christian.

Also, remove circular header file include.

Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c    | 23 +++++++++++-----------
   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h    |  7 ++-----
   4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7d1e528cc783..23bd81a76570 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1249,5 +1249,10 @@ _name##_show(struct device *dev,                                      \
                                                                      \
   static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)

+static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
+{
+    return adev->tmz.enabled;
+}
+
   #endif

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4eee40b9d0b0..f12b817480bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1058,7 +1058,7 @@ 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);
+    amdgpu_tmz_set(adev);

      return ret;
   }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
index 823527a0fa47..518b9d335550 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
@@ -27,26 +27,25 @@
   #include "amdgpu.h"
   #include "amdgpu_tmz.h"

-
   /**
- * amdgpu_is_tmz - validate trust memory zone
- *
+ * amdgpu_tmz_set -- check and set if a device supports TMZ
    * @adev: amdgpu_device pointer
    *
- * Return true if @dev supports trusted memory zones (TMZ), and return false if
- * @dev does not support TMZ.
+ * Check and set if an the device @adev supports Trusted Memory
+ * Zones (TMZ).
    */
-bool amdgpu_is_tmz(struct amdgpu_device *adev)
+void amdgpu_tmz_set(struct amdgpu_device *adev)
   {
      if (!amdgpu_tmz)
-            return false;
+            return;

-    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;
+    if (adev->asic_type < CHIP_RAVEN ||
+        adev->asic_type == CHIP_ARCTURUS) {
+            dev_warn(adev->dev, "Trusted Memory Zone (TMZ) feature not supported\n");
+            return;
      }

-    dev_info(adev->dev, "TMZ feature is enabled\n");
+    adev->tmz.enabled = true;

-    return true;
+    dev_info(adev->dev, "Trusted Memory Zone (TMZ) feature supported and enabled\n");
   }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
index 28e05177fb89..ad3ad8c011f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
@@ -24,16 +24,13 @@
   #ifndef __AMDGPU_TMZ_H__
   #define __AMDGPU_TMZ_H__

-#include "amdgpu.h"
-
   /*
- * Trust memory zone stuff
+ * Trusted Memory Zone particulars
    */
   struct amdgpu_tmz {
      bool    enabled;
   };

-
-extern bool amdgpu_is_tmz(struct amdgpu_device *adev);
+extern void amdgpu_tmz_set(struct amdgpu_device *adev);

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

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux