[PATCH] drm/amdgpu: implement TMZ accessor
Christian König
ckoenig.leichtzumerken at gmail.com
Sun Nov 3 18:24:17 UTC 2019
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 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.
> 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 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
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list