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@xxxxxxx>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
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@xxxxxx>
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;
> }
>
