[AMD Official Use Only] >From a6512c0897aa58ccac9e5483d31193d83fb590b2 Mon Sep 17 00:00:00 2001 From: Marina Nikolic <[email protected]> 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 <[email protected]> --- 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 <[email protected]> Sent: Wednesday, December 22, 2021 4:19 AM To: Nikolic, Marina <[email protected]>; Russell, Kent <[email protected]>; [email protected] <[email protected]> Cc: Mitrovic, Milan <[email protected]>; Kitchen, Greg <[email protected]> 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 <[email protected]> On Behalf Of Nikolic, Marina Sent: Tuesday, December 21, 2021 10:36 PM To: Russell, Kent <[email protected]>; [email protected] Cc: Mitrovic, Milan <[email protected]>; Kitchen, Greg <[email protected]> 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 <[email protected]<mailto:[email protected]>> 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 <[email protected]> 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 <[email protected]<mailto:[email protected]>> --- 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 <[email protected]<mailto:[email protected]>> Sent: Tuesday, December 21, 2021 3:15 PM To: Russell, Kent <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> <[email protected]<mailto:[email protected]>> Cc: Mitrovic, Milan <[email protected]<mailto:[email protected]>>; Kitchen, Greg <[email protected]<mailto:[email protected]>> 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:[email protected]> 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 <[email protected]<mailto:[email protected]>> Sent: Monday, December 20, 2021 8:01 PM To: Nikolic, Marina <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> <[email protected]<mailto:[email protected]>> Cc: Mitrovic, Milan <[email protected]<mailto:[email protected]>>; Nikolic, Marina <[email protected]<mailto:[email protected]>>; Kitchen, Greg <[email protected]<mailto:[email protected]>> 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 > <[email protected]<mailto:[email protected]>> > On Behalf Of Marina Nikolic > Sent: Monday, December 20, 2021 11:09 AM > To: [email protected]<mailto:[email protected]> > Cc: Mitrovic, Milan <[email protected]<mailto:[email protected]>>; > Nikolic, Marina > <[email protected]<mailto:[email protected]>>; Kitchen, Greg > <[email protected]<mailto:[email protected]>> > 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 > <[email protected]<mailto:[email protected]>> > --- > 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
