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.
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
