RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
From: amd-gfx On Behalf Of Nikolic,
Marina
Sent: Tuesday, December 21, 2021 10:36 PM
To: Russell, Kent ; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan ; Kitchen, Greg
Subject: Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
[AMD Official Use Only]
>From 06359f3be0c0b889519d6dd954fb11f31e9a15e0 Mon Sep 17 00:00:00 2001
From: Marina Nikolic mailto:marina.niko...@amd.com>>
Date: Tue, 14 Dec 2021 20:57:53 +0800
Subject: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
permission in ONEVF mode
[Quan, Evan] With the subject updated(remove the description about
pp_dpm_sclk), the patch is acked-by: Evan Quan
BR
Evan
== Description ==
Setting through sysfs should not be allowed in SRIOV mode.
These calls will not be processed by FW anyway,
but error handling on sysfs level should be improved.
== Changes ==
This patch prohibits performing of all set commands
in SRIOV mode on sysfs level.
It offers better error handling as calls that are
not allowed will not be propagated further.
== Test ==
Writing to any sysfs file in passthrough mode will succeed.
Writing to any sysfs file in ONEVF mode will yield error:
"calling process does not have sufficient permission to execute a command".
Signed-off-by: Marina Nikolic
mailto:marina.niko...@amd.com>>
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 082539c70fd4..c43818cd02aa 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
}
}
+ /* setting should not be allowed from VF */
+ if (amdgpu_sriov_vf(adev)) {
+ dev_attr->attr.mode &= ~S_IWUGO;
+ dev_attr->store = NULL;
+ }
+
#undef DEVICE_ATTR_IS
return 0;
--
2.20.1
From: Nikolic, Marina mailto:marina.niko...@amd.com>>
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent mailto:kent.russ...@amd.com>>;
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan mailto:milan.mitro...@amd.com>>;
Kitchen, Greg mailto:greg.kitc...@amd.com>>
Subject: Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
Hi Kent,
Thank you for the review. Yes, I can confirm I am trying to set this for every
single file for SRIOV mode.
@Kitchen, Greg<mailto:greg.kitc...@amd.com> required this for ROCM-SMI 5.0
release. In case you need it, he can provide more details.
I'm going to clarify commit message more and send a new patch.
BR,
Marina
From: Russell, Kent mailto:kent.russ...@amd.com>>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina mailto:marina.niko...@amd.com>>;
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan mailto:milan.mitro...@amd.com>>;
Nikolic, Marina mailto:marina.niko...@amd.com>>;
Kitchen, Greg mailto:greg.kitc...@amd.com>>
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
> -Original Message-
> From: amd-gfx
> mailto:amd-gfx-boun...@lists.freedesktop.org>>
> On Behalf Of Marina Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> Cc: Mitrovic, Milan mailto:milan.mitro...@amd.com>>;
> Nikolic, Marina
> mailto:marina.niko...@amd.com>>; Kitchen, Greg
> mailto:greg.kitc...@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 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
Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
>From 06359f3be0c0b889519d6dd954fb11f31e9a15e0 Mon Sep 17 00:00:00 2001
From: Marina Nikolic
Date: Tue, 14 Dec 2021 20:57:53 +0800
Subject: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
permission in ONEVF mode
== Description ==
Setting through sysfs should not be allowed in SRIOV mode.
These calls will not be processed by FW anyway,
but error handling on sysfs level should be improved.
== Changes ==
This patch prohibits performing of all set commands
in SRIOV mode on sysfs level.
It offers better error handling as calls that are
not allowed will not be propagated further.
== Test ==
Writing to any sysfs file in passthrough mode will succeed.
Writing to any sysfs file in ONEVF mode will yield error:
"calling process does not have sufficient permission to execute a command".
Signed-off-by: Marina Nikolic
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 082539c70fd4..c43818cd02aa 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
}
}
+ /* setting should not be allowed from VF */
+ if (amdgpu_sriov_vf(adev)) {
+ dev_attr->attr.mode &= ~S_IWUGO;
+ dev_attr->store = NULL;
+ }
+
#undef DEVICE_ATTR_IS
return 0;
--
2.20.1
From: Nikolic, Marina
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent ; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan ; Kitchen, Greg
Subject: Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
Hi Kent,
Thank you for the review. Yes, I can confirm I am trying to set this for every
single file for SRIOV mode.
@Kitchen, Greg<mailto:greg.kitc...@amd.com> required this for ROCM-SMI 5.0
release. In case you need it, he can provide more details.
I'm going to clarify commit message more and send a new patch.
BR,
Marina
From: Russell, Kent
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina ; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan ; Nikolic, Marina
; Kitchen, Greg
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
> -Original Message-
> From: amd-gfx On Behalf Of Marina
> Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan ; Nikolic, Marina
> ; Kitchen, Greg
> 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 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
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 082539c70fd4..d2b168babc7d 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
> *adev,
> struct amdgpu_device_
> }
> }
>
> + /* security: setting should not be allowed from VF */
> + if (amdgpu_sriov_vf(adev)) {
You should be checking for pp_dpm_sclk here, for example:
if (DEVICE_ATTR_IS(pp_dpm_sclk) {
Otherwise I am pretty sure you're setting this for every single file. And is it
only sclk? Or does it also need to affect mclk/fclk/etc? If it's only sclk, the
line above should help. If it's for more, then the commit should try to clarify
that as it's not 100% clear.
Kent
> + dev_attr->attr.mode &= ~S_IWUGO;
> + dev_attr->store = NULL;
> + }
> +
> #undef DEVICE_ATTR_IS
>
> return 0;
> --
> 2.20.1
Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
Hi Kent,
Thank you for the review. Yes, I can confirm I am trying to set this for every
single file for SRIOV mode.
@Kitchen, Greg<mailto:greg.kitc...@amd.com> required this for ROCM-SMI 5.0
release. In case you need it, he can provide more details.
I'm going to clarify commit message more and send a new patch.
BR,
Marina
From: Russell, Kent
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina ; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan ; Nikolic, Marina
; Kitchen, Greg
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
> -Original Message-
> From: amd-gfx On Behalf Of Marina
> Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan ; Nikolic, Marina
> ; Kitchen, Greg
> 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 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
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 082539c70fd4..d2b168babc7d 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
> *adev,
> struct amdgpu_device_
> }
> }
>
> + /* security: setting should not be allowed from VF */
> + if (amdgpu_sriov_vf(adev)) {
You should be checking for pp_dpm_sclk here, for example:
if (DEVICE_ATTR_IS(pp_dpm_sclk) {
Otherwise I am pretty sure you're setting this for every single file. And is it
only sclk? Or does it also need to affect mclk/fclk/etc? If it's only sclk, the
line above should help. If it's for more, then the commit should try to clarify
that as it's not 100% clear.
Kent
> + dev_attr->attr.mode &= ~S_IWUGO;
> + dev_attr->store = NULL;
> + }
> +
> #undef DEVICE_ATTR_IS
>
> return 0;
> --
> 2.20.1
RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
> -Original Message-
> From: amd-gfx On Behalf Of Marina
> Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan ; Nikolic, Marina
> ; Kitchen, Greg
> 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 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
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 082539c70fd4..d2b168babc7d 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
> *adev,
> struct amdgpu_device_
> }
> }
>
> + /* security: setting should not be allowed from VF */
> + if (amdgpu_sriov_vf(adev)) {
You should be checking for pp_dpm_sclk here, for example:
if (DEVICE_ATTR_IS(pp_dpm_sclk) {
Otherwise I am pretty sure you're setting this for every single file. And is it
only sclk? Or does it also need to affect mclk/fclk/etc? If it's only sclk, the
line above should help. If it's for more, then the commit should try to clarify
that as it's not 100% clear.
Kent
> + dev_attr->attr.mode &= ~S_IWUGO;
> + dev_attr->store = NULL;
> + }
> +
> #undef DEVICE_ATTR_IS
>
> return 0;
> --
> 2.20.1
Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
Hello,
Thank you all for fast response and useful advice.
I made new changes based on suggestion from @Lazar,
Lijo<mailto:lijo.la...@amd.com>.
Though, I have made it general (any setting in sriov case) as that is in my
request (details: SWDEV-313004).
Please find the new patch below.
Thank you and best regards,
Marina
>From 9afe76f6e4036143bed0047d71c05069fdeb44ee Mon Sep 17 00:00:00 2001
From: Marina Nikolic
Date: Tue, 14 Dec 2021 20:57:53 +0800
Subject: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
permission 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 mechanism 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 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
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 082539c70fd4..d2b168babc7d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2133,6 +2133,12 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
}
}
+ /* security: setting should not be allowed from VF */
+ if (amdgpu_sriov_vf(adev)) {
+ dev_attr->attr.mode &= ~S_IWUGO;
+ dev_attr->store = NULL;
+ }
+
#undef DEVICE_ATTR_IS
return 0;
--
2.20.1
From: Lazar, Lijo
Sent: Tuesday, December 14, 2021 5:36 AM
To: Quan, Evan ; Kasiviswanathan, Harish
; Nikolic, Marina ;
Deng, Emily
Cc: Kitchen, Greg
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
Ah, just noticed that below code won't compile (misses one '{'). Regardless,
hope you get the idea.
Thanks,
Lijo
-Original Message-
From: Lazar, Lijo
Sent: Tuesday, December 14, 2021 10:04 AM
To: Quan, Evan ; Kasiviswanathan, Harish
; Nikolic, Marina ;
Deng, Emily
Cc: Kitchen, Greg
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
Hi Marina,
attr_update is the recommended method to dynamically update the attribute
properties.
If it's only for sclk, you may do as below (I guess, passthrough and VF are
mutually exclusive).
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2132,6 +2132,13 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
dev_attr->store = NULL;
}
}
+ if (DEVICE_ATTR_IS(pp_dpm_sclk)) {
+ /* Don't allow to set GFX clock in VF mode*/
+ if (amdgpu_sriov_vf(adev))
+ dev_attr->attr.mode &= ~S_IWUGO;
+ dev_attr->store = NULL;
+ }
+ }
Thanks,
Lijo
-Original Message-
From: Quan, Evan
Sent: Tuesday, December 14, 2021 8:32 AM
To: Kasiviswanathan, Harish ; Nikolic, Marina
; Lazar, Lijo ; Deng, Emily
Cc: Kitchen, Greg
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read
premission in ONEVF mode
[AMD Official Use Only]
+Emily from SRIOV team
I think driver uses the Macros below to tell about the ONEVF and PASSTHROUGH
support:
#define amdgpu_sriov_is_pp_one_vf(adev) \
((adev)->virt.gim_feature & AMDGIM_FEATURE_PP_ONE_VF)
#define amdgpu_passthrough(adev) \
((adev)->virt.caps & AMDGPU_PASSTHROUGH_MODE)
@Deng, Emily please correct me if anything wrong.
BR
Evan
> -Original Message-
> From: Kasiviswanathan, Harish
> Sent: Tuesday, December 14, 2021 1:50 AM
> To: Kasiviswanathan, Harish ; Nikolic,
> Marina ; Quan, Evan ;
> Lazar, Lijo
> Cc: Kitchen, Greg
> Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only
> read premission in ONEVF mode
>
> [AMD Official Use Only]
>
> Hi Lijo and Evan,
>
> Could you please advice Marina on how to go about this? Does driver
> have a mechanism to distinguish between ONEVF and PASSTHROUGH case?
>
> Best Regards,
> Harish
>
>
> -Original Message-
> From: amd-gfx On Behalf Of
> Kasiviswanathan, Harish
>
RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
Some comment inline.
-Original Message-
From: amd-gfx On Behalf Of Marina
Nikolic
Sent: Friday, December 10, 2021 10:06 AM
To: amd-gfx@lists.freedesktop.org
Cc: Nikolic, Marina
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
---
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
Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
+Greg
From: Marina Nikolic
Sent: Friday, December 10, 2021 4:05 PM
To: amd-gfx@lists.freedesktop.org
Cc: Nikolic, Marina ; Nikolic, Marina
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 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
---
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),
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
Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
Please ignore this one.
There was some issue, and all changes are not included in the patch.
I have sent an update in a new mail (same subject).
Sorry for inconvenience.
BR,
Marina
From: Marina Nikolic
Sent: Friday, December 10, 2021 1:00 PM
To: amd-gfx@lists.freedesktop.org
Cc: Nikolic, Marina ; Nikolic, Marina
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 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
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 +-
drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 082539c70fd4..a78dd0799492 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),
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),
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
