[PATCH] amdgpu/pm: Modify sysfs to have only read permission in SRIOV/ONEVF mode
Lazar, Lijo
lijo.lazar at amd.com
Wed Dec 22 13:57:33 UTC 2021
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 at 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 at 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 at 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 at amd.com>
> *Sent:* Wednesday, December 22, 2021 4:19 AM
> *To:* Nikolic, Marina <Marina.Nikolic at amd.com>; Russell, Kent
> <Kent.Russell at amd.com>; amd-gfx at lists.freedesktop.org
> <amd-gfx at lists.freedesktop.org>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic at amd.com>; Kitchen, Greg
> <Greg.Kitchen at 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 at lists.freedesktop.org> *On Behalf Of *
> Nikolic, Marina
> *Sent:* Tuesday, December 21, 2021 10:36 PM
> *To:* Russell, Kent <Kent.Russell at amd.com>; amd-gfx at lists.freedesktop.org
> *Cc:* Mitrovic, Milan <Milan.Mitrovic at amd.com>; Kitchen, Greg
> <Greg.Kitchen at 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 at amd.com
> <mailto:Marina.Nikolic at 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 at 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 at amd.com
> <mailto:Marina.Nikolic at 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 at amd.com
> <mailto:Marina.Nikolic at amd.com>>
> *Sent:* Tuesday, December 21, 2021 3:15 PM
> *To:* Russell, Kent <Kent.Russell at amd.com
> <mailto:Kent.Russell at amd.com>>; amd-gfx at lists.freedesktop.org
> <mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org
> <mailto:amd-gfx at lists.freedesktop.org>>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic at amd.com
> <mailto:Milan.Mitrovic at amd.com>>; Kitchen, Greg <Greg.Kitchen at amd.com
> <mailto:Greg.Kitchen at 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 at 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 at amd.com <mailto:Kent.Russell at amd.com>>
> *Sent:* Monday, December 20, 2021 8:01 PM
> *To:* Nikolic, Marina <Marina.Nikolic at amd.com
> <mailto:Marina.Nikolic at amd.com>>; amd-gfx at lists.freedesktop.org
> <mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org
> <mailto:amd-gfx at lists.freedesktop.org>>
> *Cc:* Mitrovic, Milan <Milan.Mitrovic at amd.com
> <mailto:Milan.Mitrovic at amd.com>>; Nikolic, Marina
> <Marina.Nikolic at amd.com <mailto:Marina.Nikolic at amd.com>>; Kitchen, Greg
> <Greg.Kitchen at amd.com <mailto:Greg.Kitchen at 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 at lists.freedesktop.org
> <mailto:amd-gfx-bounces at lists.freedesktop.org>> On Behalf Of Marina Nikolic
>> Sent: Monday, December 20, 2021 11:09 AM
>> To: amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> Cc: Mitrovic, Milan <Milan.Mitrovic at amd.com <mailto:Milan.Mitrovic at amd.com>>; Nikolic, Marina
>> <Marina.Nikolic at amd.com <mailto:Marina.Nikolic at amd.com>>; Kitchen, Greg
> <Greg.Kitchen at amd.com <mailto:Greg.Kitchen at amd.com>>
>> Subject: [PATCH] amdgpu/pm: Modify sysfs pp_dpm_sclk to have only read premission in
>> ONEVF mode
>>
>> == Description ==
>> Due to security reasons setting through sysfs
>> should only be allowed in passthrough mode.
>> Options that are not mapped as SMU messages
>> do not have any mechanizm to distinguish between
>> passthorugh, onevf and mutivf usecase.
>> A unified approach is needed.
>>
>> == Changes ==
>> This patch introduces a new mechanizm to distinguish
>> ONEVF and PASSTHROUGH use case on sysfs level
>> and prohibit setting (writting to sysfs).
>> It also applies the new mechanizm on pp_dpm_sclk sysfs file.
>>
>> == Test ==
>> Writing to pp_dpm_sclk sysfs file in passthrough mode will succeed.
>> Writing to pp_dpm_sclk sysfs file in ONEVF mode will yield error:
>> "calling process does not have sufficient permission to execute a command".
>> Sysfs pp_dpm_sclk will not be created in MULTIVF mode.
>>
>> Signed-off-by: Marina Nikolic <Marina.Nikolic at amd.com <mailto:Marina.Nikolic at 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
>
More information about the amd-gfx
mailing list