[PATCH] drm/amdgpu: implement TMZ accessor

[Alex Deucher]

On Thu, Oct 31, 2019 at 8:15 PM Tuikov, Luben <Luben.Tuikov at amd.com> 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.

Alex

> Regards,
> Luben
>
> >
> > Regards,
> > Christian.
> >
> >>
> >> Also, remove circular header file include.
> >>
> >> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
> >> ---
> >>   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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx