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

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

 



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.

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




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

  Powered by Linux