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
