* [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
@ 2021-12-20 16:08 Marina Nikolic
2021-12-20 19:01 ` Russell, Kent
0 siblings, 1 reply; 14+ messages in thread
From: Marina Nikolic @ 2021-12-20 16:08 UTC (permalink / raw)
To: amd-gfx; +Cc: milan.mitrovic, Marina Nikolic, greg.kitchen
== 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 <Marina.Nikolic@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..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
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-20 16:08 [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode Marina Nikolic
@ 2021-12-20 19:01 ` Russell, Kent
2021-12-21 14:15 ` Nikolic, Marina
0 siblings, 1 reply; 14+ messages in thread
From: Russell, Kent @ 2021-12-20 19:01 UTC (permalink / raw)
To: Nikolic, Marina, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg, Nikolic, Marina
[AMD Official Use Only]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Marina Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Nikolic, Marina
> <Marina.Nikolic@amd.com>; Kitchen, Greg <Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@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..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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-20 19:01 ` Russell, Kent
@ 2021-12-21 14:15 ` Nikolic, Marina
2021-12-21 14:36 ` Nikolic, Marina
0 siblings, 1 reply; 14+ messages in thread
From: Nikolic, Marina @ 2021-12-21 14:15 UTC (permalink / raw)
To: Russell, Kent, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 3287 bytes --]
[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.Kitchen@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 <Kent.Russell@amd.com>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina <Marina.Nikolic@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Nikolic, Marina <Marina.Nikolic@amd.com>; Kitchen, Greg <Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Marina Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Nikolic, Marina
> <Marina.Nikolic@amd.com>; Kitchen, Greg <Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@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..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
[-- Attachment #2: Type: text/html, Size: 5896 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-21 14:15 ` Nikolic, Marina
@ 2021-12-21 14:36 ` Nikolic, Marina
2021-12-22 3:19 ` Quan, Evan
0 siblings, 1 reply; 14+ messages in thread
From: Nikolic, Marina @ 2021-12-21 14:36 UTC (permalink / raw)
To: Russell, Kent, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 5303 bytes --]
[AMD Official Use Only]
From 06359f3be0c0b889519d6dd954fb11f31e9a15e0 Mon Sep 17 00:00:00 2001
From: Marina Nikolic <Marina.Nikolic@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
== 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 <Marina.Nikolic@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 <Marina.Nikolic@amd.com>
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg <Greg.Kitchen@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.Kitchen@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 <Kent.Russell@amd.com>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina <Marina.Nikolic@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Nikolic, Marina <Marina.Nikolic@amd.com>; Kitchen, Greg <Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Marina Nikolic
> Sent: Monday, December 20, 2021 11:09 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Nikolic, Marina
> <Marina.Nikolic@amd.com>; Kitchen, Greg <Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@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..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
[-- Attachment #2: Type: text/html, Size: 9184 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-21 14:36 ` Nikolic, Marina
@ 2021-12-22 3:19 ` Quan, Evan
2021-12-22 11:25 ` [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode Nikolic, Marina
0 siblings, 1 reply; 14+ messages in thread
From: Quan, Evan @ 2021-12-22 3:19 UTC (permalink / raw)
To: Nikolic, Marina, Russell, Kent, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 6586 bytes --]
[AMD Official Use Only]
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Nikolic, Marina
Sent: Tuesday, December 21, 2021 10:36 PM
To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg <Greg.Kitchen@amd.com>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <evan.quan@amd.com>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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.Kitchen@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 <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@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 <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina
> <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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..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
[-- Attachment #2: Type: text/html, Size: 17914 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode
2021-12-22 3:19 ` Quan, Evan
@ 2021-12-22 11:25 ` Nikolic, Marina
2021-12-22 13:57 ` Lazar, Lijo
2021-12-23 2:31 ` Quan, Evan
0 siblings, 2 replies; 14+ messages in thread
From: Nikolic, Marina @ 2021-12-22 11:25 UTC (permalink / raw)
To: Quan, Evan, Russell, Kent, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 8803 bytes --]
[AMD Official Use Only]
From a6512c0897aa58ccac9e5483d31193d83fb590b2 Mon Sep 17 00:00:00 2001
From: Marina Nikolic <Marina.Nikolic@amd.com>
Date: Tue, 14 Dec 2021 20:57:53 +0800
Subject: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in
SRIOV/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 <Marina.Nikolic@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: Quan, Evan <Evan.Quan@amd.com>
Sent: Wednesday, December 22, 2021 4:19 AM
To: Nikolic, Marina <Marina.Nikolic@amd.com>; Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg <Greg.Kitchen@amd.com>
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Nikolic, Marina
Sent: Tuesday, December 21, 2021 10:36 PM
To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg <Greg.Kitchen@amd.com>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <evan.quan@amd.com>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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.Kitchen@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 <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@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 <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina
> <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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..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
[-- Attachment #2: Type: text/html, Size: 24403 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode
2021-12-22 11:25 ` [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode Nikolic, Marina
@ 2021-12-22 13:57 ` Lazar, Lijo
2021-12-23 2:31 ` Quan, Evan
1 sibling, 0 replies; 14+ messages in thread
From: Lazar, Lijo @ 2021-12-22 13:57 UTC (permalink / raw)
To: Nikolic, Marina, Quan, Evan, Russell, Kent, amd-gfx
Cc: Mitrovic, Milan, Kitchen, Greg
On 12/22/2021 4:55 PM, Nikolic, Marina wrote:
> [AMD Official Use Only]
>
>
> [AMD Official Use Only]
>
>
> From a6512c0897aa58ccac9e5483d31193d83fb590b2 Mon Sep 17 00:00:00 2001
> From: Marina Nikolic <Marina.Nikolic@amd.com>
> Date: Tue, 14 Dec 2021 20:57:53 +0800
> Subject: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in
> SRIOV/ONEVF mode
>
Maybe change subject as "Make sysfs pm attributes as read-only for VFs"
and description like as "Setting values of pm attributes through sysfs..."
Only cosmetic changes, no need to post another one for review again.
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
> == 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 <Marina.Nikolic@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:* Quan, Evan <Evan.Quan@amd.com>
> *Sent:* Wednesday, December 22, 2021 4:19 AM
> *To:* Nikolic, Marina <Marina.Nikolic@amd.com>; Russell, Kent
> <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> <amd-gfx@lists.freedesktop.org>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg
> <Greg.Kitchen@amd.com>
> *Subject:* RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only
> read premission in ONEVF mode
>
> [AMD Official Use Only]
>
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> *On Behalf Of *
> Nikolic, Marina
> *Sent:* Tuesday, December 21, 2021 10:36 PM
> *To:* Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> *Cc:* Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg
> <Greg.Kitchen@amd.com>
> *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 <Marina.Nikolic@amd.com
> <mailto:Marina.Nikolic@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 <evan.quan@amd.com>/*
>
> 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 <Marina.Nikolic@amd.com
> <mailto:Marina.Nikolic@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 <Marina.Nikolic@amd.com
> <mailto:Marina.Nikolic@amd.com>>
> *Sent:* Tuesday, December 21, 2021 3:15 PM
> *To:* Russell, Kent <Kent.Russell@amd.com
> <mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic@amd.com
> <mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com
> <mailto:Greg.Kitchen@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.Kitchen@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 <Kent.Russell@amd.com <mailto:Kent.Russell@amd.com>>
> *Sent:* Monday, December 20, 2021 8:01 PM
> *To:* Nikolic, Marina <Marina.Nikolic@amd.com
> <mailto:Marina.Nikolic@amd.com>>; amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic@amd.com
> <mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina
> <Marina.Nikolic@amd.com <mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg
> <Greg.Kitchen@amd.com <mailto:Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org
> <mailto:amd-gfx-bounces@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 <Milan.Mitrovic@amd.com <mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina
>> <Marina.Nikolic@amd.com <mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg
> <Greg.Kitchen@amd.com <mailto:Greg.Kitchen@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 mode.
>>
>> Signed-off-by: Marina Nikolic <Marina.Nikolic@amd.com <mailto:Marina.Nikolic@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..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
>
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode
2021-12-22 11:25 ` [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode Nikolic, Marina
2021-12-22 13:57 ` Lazar, Lijo
@ 2021-12-23 2:31 ` Quan, Evan
1 sibling, 0 replies; 14+ messages in thread
From: Quan, Evan @ 2021-12-23 2:31 UTC (permalink / raw)
To: Nikolic, Marina, Russell, Kent, amd-gfx; +Cc: Mitrovic, Milan, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 9739 bytes --]
[AMD Official Use Only]
Reviewed-by: Evan Quan <evan.quan@amd.com>
From: Nikolic, Marina <Marina.Nikolic@amd.com>
Sent: Wednesday, December 22, 2021 7:25 PM
To: Quan, Evan <Evan.Quan@amd.com>; Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com>; Kitchen, Greg <Greg.Kitchen@amd.com>
Subject: Re: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode
[AMD Official Use Only]
From a6512c0897aa58ccac9e5483d31193d83fb590b2 Mon Sep 17 00:00:00 2001
From: Marina Nikolic <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>
Date: Tue, 14 Dec 2021 20:57:53 +0800
Subject: [PATCH] amdgpu/pm: Modify sysfs to have only read permission in
SRIOV/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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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: Quan, Evan <Evan.Quan@amd.com<mailto:Evan.Quan@amd.com>>
Sent: Wednesday, December 22, 2021 4:19 AM
To: Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@amd.com>>
Subject: RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
[AMD Official Use Only]
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> On Behalf Of Nikolic, Marina
Sent: Tuesday, December 21, 2021 10:36 PM
To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@amd.com>>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <evan.quan@amd.com<mailto:evan.quan@amd.com>>
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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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 <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>
Sent: Tuesday, December 21, 2021 3:15 PM
To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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.Kitchen@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 <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>
Sent: Monday, December 20, 2021 8:01 PM
To: Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Mitrovic, Milan <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@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 <Milan.Mitrovic@amd.com<mailto:Milan.Mitrovic@amd.com>>; Nikolic, Marina
> <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@amd.com>>; Kitchen, Greg <Greg.Kitchen@amd.com<mailto:Greg.Kitchen@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 mode.
>
> Signed-off-by: Marina Nikolic <Marina.Nikolic@amd.com<mailto:Marina.Nikolic@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..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
[-- Attachment #2: Type: text/html, Size: 27775 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
@ 2021-12-10 15:05 Marina Nikolic
2021-12-13 14:54 ` Nikolic, Marina
2021-12-13 17:40 ` Kasiviswanathan, Harish
0 siblings, 2 replies; 14+ messages in thread
From: Marina Nikolic @ 2021-12-10 15:05 UTC (permalink / raw)
To: amd-gfx; +Cc: Marina Nikolic
== 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 <marina.nikolic@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),
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-10 15:05 [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode Marina Nikolic
@ 2021-12-13 14:54 ` Nikolic, Marina
2021-12-13 17:40 ` Kasiviswanathan, Harish
1 sibling, 0 replies; 14+ messages in thread
From: Nikolic, Marina @ 2021-12-13 14:54 UTC (permalink / raw)
To: amd-gfx, Kitchen, Greg
[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]
[AMD Official Use Only]
+Greg
________________________________
From: Marina Nikolic <Marina.Nikolic@amd.com>
Sent: Friday, December 10, 2021 4:05 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Nikolic, Marina <Marina.Nikolic@amd.com>; Nikolic, Marina <Marina.Nikolic@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 mode.
Signed-off-by: Marina Nikolic <marina.nikolic@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),
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
[-- Attachment #2: Type: text/html, Size: 7281 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-10 15:05 [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode Marina Nikolic
2021-12-13 14:54 ` Nikolic, Marina
@ 2021-12-13 17:40 ` Kasiviswanathan, Harish
[not found] ` <DM6PR12MB404216720C63071561C3675F8C749@DM6PR12MB4042.namprd12.prod.outlook.com>
1 sibling, 1 reply; 14+ messages in thread
From: Kasiviswanathan, Harish @ 2021-12-13 17:40 UTC (permalink / raw)
To: Nikolic, Marina, amd-gfx; +Cc: Nikolic, Marina
[AMD Official Use Only]
Some comment inline.
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Marina Nikolic
Sent: Friday, December 10, 2021 10:06 AM
To: amd-gfx@lists.freedesktop.org
Cc: Nikolic, Marina <Marina.Nikolic@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@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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
@ 2021-12-10 12:00 Marina Nikolic
2021-12-10 15:19 ` Nikolic, Marina
0 siblings, 1 reply; 14+ messages in thread
From: Marina Nikolic @ 2021-12-10 12:00 UTC (permalink / raw)
To: amd-gfx; +Cc: Marina Nikolic
== 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 <marina.nikolic@amd.com>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode
2021-12-10 12:00 Marina Nikolic
@ 2021-12-10 15:19 ` Nikolic, Marina
0 siblings, 0 replies; 14+ messages in thread
From: Nikolic, Marina @ 2021-12-10 15:19 UTC (permalink / raw)
To: amd-gfx
[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]
[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 <Marina.Nikolic@amd.com>
Sent: Friday, December 10, 2021 1:00 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Nikolic, Marina <Marina.Nikolic@amd.com>; Nikolic, Marina <Marina.Nikolic@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 mode.
Signed-off-by: Marina Nikolic <marina.nikolic@amd.com>
---
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
[-- Attachment #2: Type: text/html, Size: 7284 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-12-23 2:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 16:08 [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode Marina Nikolic
2021-12-20 19:01 ` Russell, Kent
2021-12-21 14:15 ` Nikolic, Marina
2021-12-21 14:36 ` Nikolic, Marina
2021-12-22 3:19 ` Quan, Evan
2021-12-22 11:25 ` [PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode Nikolic, Marina
2021-12-22 13:57 ` Lazar, Lijo
2021-12-23 2:31 ` Quan, Evan
-- strict thread matches above, loose matches on Subject: below --
2021-12-10 15:05 [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in ONEVF mode Marina Nikolic
2021-12-13 14:54 ` Nikolic, Marina
2021-12-13 17:40 ` Kasiviswanathan, Harish
[not found] ` <DM6PR12MB404216720C63071561C3675F8C749@DM6PR12MB4042.namprd12.prod.outlook.com>
[not found] ` <DM6PR12MB2619DFBA6CC70A93C3547BECE4759@DM6PR12MB2619.namprd12.prod.outlook.com>
[not found] ` <DM6PR12MB393064EE17A327FCA213831897759@DM6PR12MB3930.namprd12.prod.outlook.com>
[not found] ` <DM6PR12MB3930E43AA72957083762380D97759@DM6PR12MB3930.namprd12.prod.outlook.com>
2021-12-14 13:14 ` Nikolic, Marina
2021-12-10 12:00 Marina Nikolic
2021-12-10 15:19 ` Nikolic, Marina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.