[PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode

[Kasiviswanathan, Harish]

[AMD Official Use Only]

Some comment inline.

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Marina Nikolic
Sent: Friday, December 10, 2021 10:06 AM
To: amd-gfx at lists.freedesktop.org
Cc: Nikolic, Marina <Marina.Nikolic at amd.com>
Subject: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode

== Description ==
Due to security reasons setting through sysfs
should only be allowed in passthrough mode.
Options that are not mapped as SMU messages
do not have any mechanizm to distinguish between
passthorugh, onevf and mutivf usecase.
A unified approach is needed.

== Changes ==
This patch introduces a new mechanizm to distinguish
ONEVF and PASSTHROUGH 

[HK]: It is not clear how you distinguish between ONEVF and PASSTHROUGH. Could you please elaborate? 

use case on sysfs level
and prohibit setting (writting to sysfs).
It also applies the new mechanizm on pp_dpm_sclk sysfs file.

== Test ==
Writing to pp_dpm_sclk sysfs file in passthrough mode will succeed.
Writing to pp_dpm_sclk sysfs file in ONEVF mode will yield error:
"calling process does not have sufficient permission to execute a command".
Sysfs pp_dpm_sclk will not be created in MULTIVF mode.

Signed-off-by: Marina Nikolic <marina.nikolic at amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c     | 4 ++--
 drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 082539c70fd4..0ccc23ee76a8 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2021,7 +2021,7 @@ static struct amdgpu_device_attr amdgpu_device_attrs[] = {
 	AMDGPU_DEVICE_ATTR_RO(pp_cur_state,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
 	AMDGPU_DEVICE_ATTR_RW(pp_force_state,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
 	AMDGPU_DEVICE_ATTR_RW(pp_table,					ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
-	AMDGPU_DEVICE_ATTR_RW(pp_dpm_sclk,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
+	AMDGPU_DEVICE_ATTR_RRW(pp_dpm_sclk,				ATTR_FLAG_BASIC, ATTR_FLAG_ONEVF),

[HK]: Wouldn't this macro try to create two sysfs entries with same name. The second time the function would fail. 

 	AMDGPU_DEVICE_ATTR_RW(pp_dpm_mclk,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
 	AMDGPU_DEVICE_ATTR_RW(pp_dpm_socclk,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
 	AMDGPU_DEVICE_ATTR_RW(pp_dpm_fclk,				ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
@@ -3504,7 +3504,7 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
 		break;
 	case SRIOV_VF_MODE_BARE_METAL:
 	default:
-		mask = ATTR_FLAG_MASK_ALL;
+		mask = ATTR_FLAG_BASIC;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
index a920515e2274..1a30d9c48d13 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
@@ -79,6 +79,10 @@ struct amdgpu_device_attr_entry {
 			     amdgpu_get_##_name, NULL,			\
 			     _flags, ##__VA_ARGS__)
 
+#define AMDGPU_DEVICE_ATTR_RRW(_name, _flags_full, _flags_restricted, ...) 	\
+        AMDGPU_DEVICE_ATTR_RW(_name, _flags_full, ##__VA_ARGS__),		\
+        AMDGPU_DEVICE_ATTR_RO(_name, _flags_restricted, ##__VA_ARGS__)
+
 int amdgpu_pm_sysfs_init(struct amdgpu_device *adev);
 int amdgpu_pm_virt_sysfs_init(struct amdgpu_device *adev);
 void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev);
-- 
2.20.1