Re: [PATCH 4/4] drm/amdkfd: Check against device cgroup

[Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>]

On Wed, May 01, 2019 at 02:59:29PM +0000, Kasiviswanathan, Harish wrote:
> Participate in device cgroup. All kfd devices are exposed via /dev/kfd.
> So use /dev/dri/renderN node.
> 
> Before exposing the device to a task check if it has permission to
> access it. If the task (based on its cgroup) can access /dev/dri/renderN
> then expose the device via kfd node.
> 
> If the task cannot access /dev/dri/renderN then process device data
> (pdd) is not created. This will ensure that task cannot use the device.
> 
> In sysfs topology, all device nodes are visible irrespective of the task
> cgroup. The sysfs node directories are created at driver load time and
> cannot be changed dynamically. However, access to information inside
> nodes is controlled based on the task's cgroup permissions.
> 
> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

Hello, Harish!

Cgroup/device controller part looks good to me.
Please, feel free to use my acks for patches 3 and 4:
Acked-by: Roman Gushchin <guro@fb.com>

Thanks!

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c |  9 +++++++--
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h        | 17 +++++++++++++++++
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c    | 12 ++++++++++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
> index dc7339825b5c..3804edfb4ff7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
> @@ -369,8 +369,13 @@ int kfd_init_apertures(struct kfd_process *process)
>  
>  	/*Iterating over all devices*/
>  	while (kfd_topology_enum_kfd_devices(id, &dev) == 0) {
> -		if (!dev) {
> -			id++; /* Skip non GPU devices */
> +		if (!dev || kfd_devcgroup_check_permission(dev)) {
> +			/* Skip non GPU devices and devices to which the
> +			 * current process have no access to. Access can be
> +			 * limited by placing the process in a specific
> +			 * cgroup hierarchy
                                           ^
				Probably, a missing dot here.
> +			 */
> +			id++;
>  			continue;
>  		}
>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx