* AMDKFD (AMD GPU compute) support for device cgroup.
@ 2019-09-16 18:05 Kasiviswanathan, Harish
[not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Kasiviswanathan, Harish @ 2019-09-16 18:05 UTC (permalink / raw)
To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander,
airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Hi Tejun,
Sorry. Resending these patches as I got distracted with other stuff and
couldn't follow through. As per our previous discussion the plan was to
upstream these changes (mainly patch 3 "Export devcgroup_check_permission")
through Alex Deucher's amd-staging-drm-next and Dave Airlie's drm-next trees. I
am planning to take this route. I have rebased my patches to
amd-staging-drm-next.
You and Roman acked patch 3, it will be great if I can Reviewed-by, so that the
upstream path can be smoother.
Best Regards,
Harish
***** Original note about this patch series for ref. ****
amdkfd (part of amdgpu) driver supports the AMD GPU compute stack. amdkfd
exposes only a single device /dev/kfd even if multiple AMD GPU (compute)
devices exist in a system. However, amdgpu drvier exposes a separate render
device file /dev/dri/renderDN for each device. To participate in device cgroup
amdkfd driver will rely on these rendner device files
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread[parent not found: <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>]
* [PATCH v2 1/4] drm/amdkfd: Store kfd_dev in iolink and cache properties [not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> @ 2019-09-16 18:05 ` Kasiviswanathan, Harish 2019-09-16 18:05 ` [PATCH v2 2/4] drm/amd: Pass drm_device to kfd Kasiviswanathan, Harish ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-09-16 18:05 UTC (permalink / raw) To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kasiviswanathan, Harish, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org This is required to check against cgroup permissions. Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com> --- drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 10 ++++++++++ drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index f2170f0e4334..8d0cfd391d67 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -1098,6 +1098,9 @@ static struct kfd_topology_device *kfd_assign_gpu(struct kfd_dev *gpu) { struct kfd_topology_device *dev; struct kfd_topology_device *out_dev = NULL; + struct kfd_mem_properties *mem; + struct kfd_cache_properties *cache; + struct kfd_iolink_properties *iolink; down_write(&topology_lock); list_for_each_entry(dev, &topology_device_list, list) { @@ -1111,6 +1114,13 @@ static struct kfd_topology_device *kfd_assign_gpu(struct kfd_dev *gpu) if (!dev->gpu && (dev->node_props.simd_count > 0)) { dev->gpu = gpu; out_dev = dev; + + list_for_each_entry(mem, &dev->mem_props, list) + mem->gpu = dev->gpu; + list_for_each_entry(cache, &dev->cache_props, list) + cache->gpu = dev->gpu; + list_for_each_entry(iolink, &dev->io_link_props, list) + iolink->gpu = dev->gpu; break; } } diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h index d4718d58d0f2..15843e0fc756 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h @@ -102,6 +102,7 @@ struct kfd_mem_properties { uint32_t flags; uint32_t width; uint32_t mem_clk_max; + struct kfd_dev *gpu; struct kobject *kobj; struct attribute attr; }; @@ -123,6 +124,7 @@ struct kfd_cache_properties { uint32_t cache_latency; uint32_t cache_type; uint8_t sibling_map[CRAT_SIBLINGMAP_SIZE]; + struct kfd_dev *gpu; struct kobject *kobj; struct attribute attr; }; @@ -141,6 +143,7 @@ struct kfd_iolink_properties { uint32_t max_bandwidth; uint32_t rec_transfer_size; uint32_t flags; + struct kfd_dev *gpu; struct kobject *kobj; struct attribute attr; }; -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] drm/amd: Pass drm_device to kfd [not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> 2019-09-16 18:05 ` [PATCH v2 1/4] drm/amdkfd: Store kfd_dev in iolink and cache properties Kasiviswanathan, Harish @ 2019-09-16 18:05 ` Kasiviswanathan, Harish 2019-09-16 18:05 ` [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission Kasiviswanathan, Harish 2019-09-16 18:05 ` [PATCH v2 4/4] drm/amdkfd: Check against device cgroup Kasiviswanathan, Harish 3 siblings, 0 replies; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-09-16 18:05 UTC (permalink / raw) To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kasiviswanathan, Harish, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org kfd needs drm_device to call into drm_cgroup functions Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 + drivers/gpu/drm/amd/amdkfd/kfd_device.c | 2 ++ drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index 363005526d7b..681a4a9ff51c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -204,7 +204,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev) adev->doorbell_index.last_non_cp; } - kgd2kfd_device_init(adev->kfd.dev, &gpu_resources); + kgd2kfd_device_init(adev->kfd.dev, adev->ddev, &gpu_resources); } } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h index e39c106ac634..4eb2fb85de26 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h @@ -251,6 +251,7 @@ struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd, struct pci_dev *pdev, const struct kfd2kgd_calls *f2g, unsigned int asic_type, bool vf); bool kgd2kfd_device_init(struct kfd_dev *kfd, + struct drm_device *ddev, const struct kgd2kfd_shared_resources *gpu_resources); void kgd2kfd_device_exit(struct kfd_dev *kfd); void kgd2kfd_suspend(struct kfd_dev *kfd); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c index f329b82f11d9..06461ac730d4 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c @@ -514,10 +514,12 @@ static void kfd_cwsr_init(struct kfd_dev *kfd) } bool kgd2kfd_device_init(struct kfd_dev *kfd, + struct drm_device *ddev, const struct kgd2kfd_shared_resources *gpu_resources) { unsigned int size; + kfd->ddev = ddev; kfd->mec_fw_version = amdgpu_amdkfd_get_fw_version(kfd->kgd, KGD_ENGINE_MEC1); kfd->sdma_fw_version = amdgpu_amdkfd_get_fw_version(kfd->kgd, diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index 06bb2d7a9b39..9c56ba6ec826 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -47,6 +47,8 @@ /* GPU ID hash width in bits */ #define KFD_GPU_ID_HASH_WIDTH 16 +struct drm_device; + /* Use upper bits of mmap offset to store KFD driver specific information. * BITS[63:62] - Encode MMAP type * BITS[61:46] - Encode gpu_id. To identify to which GPU the offset belongs to @@ -230,6 +232,7 @@ struct kfd_dev { const struct kfd_device_info *device_info; struct pci_dev *pdev; + struct drm_device *ddev; unsigned int id; /* topology stub index */ -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission [not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> 2019-09-16 18:05 ` [PATCH v2 1/4] drm/amdkfd: Store kfd_dev in iolink and cache properties Kasiviswanathan, Harish 2019-09-16 18:05 ` [PATCH v2 2/4] drm/amd: Pass drm_device to kfd Kasiviswanathan, Harish @ 2019-09-16 18:05 ` Kasiviswanathan, Harish [not found] ` <20190916180523.27341-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> 2019-09-16 18:05 ` [PATCH v2 4/4] drm/amdkfd: Check against device cgroup Kasiviswanathan, Harish 3 siblings, 1 reply; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-09-16 18:05 UTC (permalink / raw) To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kasiviswanathan, Harish, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org For AMD compute (amdkfd) driver. All AMD compute devices are exported via single device node /dev/kfd. As a result devices cannot be controlled individually using device cgroup. AMD compute devices will rely on its graphics counterpart that exposes /dev/dri/renderN node for each device. For each task (based on its cgroup), KFD driver will check if /dev/dri/renderN node is accessible before exposing it. Change-Id: I9ae283df550b2c122d67870b0cfa316bfbf3b614 Acked-by: Felix Kuehling <Felix.Kuehling@amd.com> Acked-by: Tejun Heo <tj@kernel.org> Acked-by: Roman Gushchin <guro@fb.com> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> --- include/linux/device_cgroup.h | 19 ++++--------------- security/device_cgroup.c | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h index 8557efe096dc..fa35b52e0002 100644 --- a/include/linux/device_cgroup.h +++ b/include/linux/device_cgroup.h @@ -12,26 +12,15 @@ #define DEVCG_DEV_ALL 4 /* this represents all devices */ #ifdef CONFIG_CGROUP_DEVICE -extern int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access); +int devcgroup_check_permission(short type, u32 major, u32 minor, + short access); #else -static inline int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static inline int devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { return 0; } #endif #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF) -static inline int devcgroup_check_permission(short type, u32 major, u32 minor, - short access) -{ - int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); - - if (rc) - return -EPERM; - - return __devcgroup_check_permission(type, major, minor, access); -} - static inline int devcgroup_inode_permission(struct inode *inode, int mask) { short type, access = 0; diff --git a/security/device_cgroup.c b/security/device_cgroup.c index dc28914fa72e..04dd29bf7f06 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -801,8 +801,8 @@ struct cgroup_subsys devices_cgrp_subsys = { * * returns 0 on success, -EPERM case the operation is not permitted */ -int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static int __devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { struct dev_cgroup *dev_cgroup; bool rc; @@ -824,3 +824,14 @@ int __devcgroup_check_permission(short type, u32 major, u32 minor, return 0; } + +int devcgroup_check_permission(short type, u32 major, u32 minor, short access) +{ + int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); + + if (rc) + return -EPERM; + + return __devcgroup_check_permission(type, major, minor, access); +} +EXPORT_SYMBOL(devcgroup_check_permission); -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20190916180523.27341-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>]
* RE: [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission [not found] ` <20190916180523.27341-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> @ 2019-09-24 15:54 ` Kasiviswanathan, Harish [not found] ` <MN2PR12MB2911F59E9B91AAD349B4E40F8C840-rweVpJHSKTpv4wpD+z6awQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-09-24 15:54 UTC (permalink / raw) To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Hi Tejun, Can you please review this? You and Roman acked this patch before. It will be great if I can Reviewed-by, so that I can upstream this through Alex Deucher's amd-staging-drm-next and Dave Airlie's drm-next trees Thanks, Harish -----Original Message----- From: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com> Sent: Monday, September 16, 2019 2:06 PM To: tj@kernel.org; Deucher, Alexander <Alexander.Deucher@amd.com>; airlied@redhat.com Cc: cgroups@vger.kernel.org; amd-gfx@lists.freedesktop.org; Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com> Subject: [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission For AMD compute (amdkfd) driver. All AMD compute devices are exported via single device node /dev/kfd. As a result devices cannot be controlled individually using device cgroup. AMD compute devices will rely on its graphics counterpart that exposes /dev/dri/renderN node for each device. For each task (based on its cgroup), KFD driver will check if /dev/dri/renderN node is accessible before exposing it. Change-Id: I9ae283df550b2c122d67870b0cfa316bfbf3b614 Acked-by: Felix Kuehling <Felix.Kuehling@amd.com> Acked-by: Tejun Heo <tj@kernel.org> Acked-by: Roman Gushchin <guro@fb.com> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> --- include/linux/device_cgroup.h | 19 ++++--------------- security/device_cgroup.c | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h index 8557efe096dc..fa35b52e0002 100644 --- a/include/linux/device_cgroup.h +++ b/include/linux/device_cgroup.h @@ -12,26 +12,15 @@ #define DEVCG_DEV_ALL 4 /* this represents all devices */ #ifdef CONFIG_CGROUP_DEVICE -extern int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access); +int devcgroup_check_permission(short type, u32 major, u32 minor, + short access); #else -static inline int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static inline int devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { return 0; } #endif #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF) -static inline int devcgroup_check_permission(short type, u32 major, u32 minor, - short access) -{ - int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); - - if (rc) - return -EPERM; - - return __devcgroup_check_permission(type, major, minor, access); -} - static inline int devcgroup_inode_permission(struct inode *inode, int mask) { short type, access = 0; diff --git a/security/device_cgroup.c b/security/device_cgroup.c index dc28914fa72e..04dd29bf7f06 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -801,8 +801,8 @@ struct cgroup_subsys devices_cgrp_subsys = { * * returns 0 on success, -EPERM case the operation is not permitted */ -int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static int __devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { struct dev_cgroup *dev_cgroup; bool rc; @@ -824,3 +824,14 @@ int __devcgroup_check_permission(short type, u32 major, u32 minor, return 0; } + +int devcgroup_check_permission(short type, u32 major, u32 minor, short access) +{ + int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); + + if (rc) + return -EPERM; + + return __devcgroup_check_permission(type, major, minor, access); +} +EXPORT_SYMBOL(devcgroup_check_permission); -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <MN2PR12MB2911F59E9B91AAD349B4E40F8C840-rweVpJHSKTpv4wpD+z6awQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>]
* Re: [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission [not found] ` <MN2PR12MB2911F59E9B91AAD349B4E40F8C840-rweVpJHSKTpv4wpD+z6awQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2019-09-24 17:13 ` Roman Gushchin 0 siblings, 0 replies; 9+ messages in thread From: Roman Gushchin @ 2019-09-24 17:13 UTC (permalink / raw) To: Kasiviswanathan, Harish Cc: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org On Tue, Sep 24, 2019 at 03:54:47PM +0000, Kasiviswanathan, Harish wrote: > Hi Tejun, > > Can you please review this? You and Roman acked this patch before. It will be great if I can Reviewed-by, so that I can upstream this through Alex Deucher's amd-staging-drm-next and Dave Airlie's drm-next trees > > Thanks, > Harish Hello, Harish! If it can help, please, feel free to use Reviewed-by: Roman Gushchin <guro@fb.com> Thanks! > > > -----Original Message----- > From: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com> > Sent: Monday, September 16, 2019 2:06 PM > To: tj@kernel.org; Deucher, Alexander <Alexander.Deucher@amd.com>; airlied@redhat.com > Cc: cgroups@vger.kernel.org; amd-gfx@lists.freedesktop.org; Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com> > Subject: [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission > > For AMD compute (amdkfd) driver. > > All AMD compute devices are exported via single device node /dev/kfd. As > a result devices cannot be controlled individually using device cgroup. > > AMD compute devices will rely on its graphics counterpart that exposes > /dev/dri/renderN node for each device. For each task (based on its > cgroup), KFD driver will check if /dev/dri/renderN node is accessible > before exposing it. > > Change-Id: I9ae283df550b2c122d67870b0cfa316bfbf3b614 > Acked-by: Felix Kuehling <Felix.Kuehling@amd.com> > Acked-by: Tejun Heo <tj@kernel.org> > Acked-by: Roman Gushchin <guro@fb.com> > Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> > --- > include/linux/device_cgroup.h | 19 ++++--------------- > security/device_cgroup.c | 15 +++++++++++++-- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h > index 8557efe096dc..fa35b52e0002 100644 > --- a/include/linux/device_cgroup.h > +++ b/include/linux/device_cgroup.h > @@ -12,26 +12,15 @@ > #define DEVCG_DEV_ALL 4 /* this represents all devices */ > > #ifdef CONFIG_CGROUP_DEVICE > -extern int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access); > +int devcgroup_check_permission(short type, u32 major, u32 minor, > + short access); > #else > -static inline int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > +static inline int devcgroup_check_permission(short type, u32 major, u32 minor, > + short access) > { return 0; } > #endif > > #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF) > -static inline int devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > -{ > - int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); > - > - if (rc) > - return -EPERM; > - > - return __devcgroup_check_permission(type, major, minor, access); > -} > - > static inline int devcgroup_inode_permission(struct inode *inode, int mask) > { > short type, access = 0; > diff --git a/security/device_cgroup.c b/security/device_cgroup.c > index dc28914fa72e..04dd29bf7f06 100644 > --- a/security/device_cgroup.c > +++ b/security/device_cgroup.c > @@ -801,8 +801,8 @@ struct cgroup_subsys devices_cgrp_subsys = { > * > * returns 0 on success, -EPERM case the operation is not permitted > */ > -int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > +static int __devcgroup_check_permission(short type, u32 major, u32 minor, > + short access) > { > struct dev_cgroup *dev_cgroup; > bool rc; > @@ -824,3 +824,14 @@ int __devcgroup_check_permission(short type, u32 major, u32 minor, > > return 0; > } > + > +int devcgroup_check_permission(short type, u32 major, u32 minor, short access) > +{ > + int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); > + > + if (rc) > + return -EPERM; > + > + return __devcgroup_check_permission(type, major, minor, access); > +} > +EXPORT_SYMBOL(devcgroup_check_permission); > -- > 2.17.1 > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] drm/amdkfd: Check against device cgroup [not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> ` (2 preceding siblings ...) 2019-09-16 18:05 ` [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission Kasiviswanathan, Harish @ 2019-09-16 18:05 ` Kasiviswanathan, Harish 3 siblings, 0 replies; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-09-16 18:05 UTC (permalink / raw) To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Deucher, Alexander, airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kasiviswanathan, Harish, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org 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. Change-Id: Ic76d4d3c4e12e288033b8d22b08ffc5a2d784e5c Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com> --- 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 481661499c9a..ae950633228c 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 + */ + id++; continue; } diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index 9c56ba6ec826..a3023f932554 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -36,6 +36,8 @@ #include <linux/seq_file.h> #include <linux/kref.h> #include <linux/sysfs.h> +#include <linux/device_cgroup.h> +#include <drm/drmP.h> #include <kgd_kfd_interface.h> #include "amd_shared.h" @@ -1048,6 +1050,21 @@ bool kfd_is_locked(void); void kfd_inc_compute_active(struct kfd_dev *dev); void kfd_dec_compute_active(struct kfd_dev *dev); +/* Cgroup Support */ +/* Check with device cgroup if @kfd device is accessible */ +static inline int kfd_devcgroup_check_permission(struct kfd_dev *kfd) +{ +#if defined(CONFIG_CGROUP_DEVICE) + struct drm_device *ddev = kfd->ddev; + + return devcgroup_check_permission(DEVCG_DEV_CHAR, ddev->driver->major, + ddev->render->index, + DEVCG_ACC_WRITE | DEVCG_ACC_READ); +#else + return 0; +#endif +} + /* Debugfs */ #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index 8d0cfd391d67..92e5867aa990 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -269,6 +269,8 @@ static ssize_t iolink_show(struct kobject *kobj, struct attribute *attr, buffer[0] = 0; iolink = container_of(attr, struct kfd_iolink_properties, attr); + if (iolink->gpu && kfd_devcgroup_check_permission(iolink->gpu)) + return -EPERM; sysfs_show_32bit_prop(buffer, "type", iolink->iolink_type); sysfs_show_32bit_prop(buffer, "version_major", iolink->ver_maj); sysfs_show_32bit_prop(buffer, "version_minor", iolink->ver_min); @@ -305,6 +307,8 @@ static ssize_t mem_show(struct kobject *kobj, struct attribute *attr, buffer[0] = 0; mem = container_of(attr, struct kfd_mem_properties, attr); + if (mem->gpu && kfd_devcgroup_check_permission(mem->gpu)) + return -EPERM; sysfs_show_32bit_prop(buffer, "heap_type", mem->heap_type); sysfs_show_64bit_prop(buffer, "size_in_bytes", mem->size_in_bytes); sysfs_show_32bit_prop(buffer, "flags", mem->flags); @@ -334,6 +338,8 @@ static ssize_t kfd_cache_show(struct kobject *kobj, struct attribute *attr, buffer[0] = 0; cache = container_of(attr, struct kfd_cache_properties, attr); + if (cache->gpu && kfd_devcgroup_check_permission(cache->gpu)) + return -EPERM; sysfs_show_32bit_prop(buffer, "processor_id_low", cache->processor_id_low); sysfs_show_32bit_prop(buffer, "level", cache->cache_level); @@ -414,6 +420,8 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr, if (strcmp(attr->name, "gpu_id") == 0) { dev = container_of(attr, struct kfd_topology_device, attr_gpuid); + if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu)) + return -EPERM; return sysfs_show_32bit_val(buffer, dev->gpu_id); } @@ -421,11 +429,15 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr, dev = container_of(attr, struct kfd_topology_device, attr_name); + if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu)) + return -EPERM; return sysfs_show_str_val(buffer, dev->node_props.name); } dev = container_of(attr, struct kfd_topology_device, attr_props); + if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu)) + return -EPERM; sysfs_show_32bit_prop(buffer, "cpu_cores_count", dev->node_props.cpu_cores_count); sysfs_show_32bit_prop(buffer, "simd_count", -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 0/4] AMDKFD (AMD GPU compute) support for device cgroup.
@ 2019-05-17 16:14 Kasiviswanathan, Harish
[not found] ` <20190517161435.14121-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Kasiviswanathan, Harish @ 2019-05-17 16:14 UTC (permalink / raw)
To: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Kasiviswanathan, Harish
amdkfd (part of amdgpu) driver supports the AMD GPU compute stack.
amdkfd exposes only a single device /dev/kfd even if multiple AMD GPU
(compute) devices exist in a system. However, amdgpu drvier exposes a
separate render device file /dev/dri/renderDN for each device. To participate
in device cgroup amdkfd driver will rely on these redner device files.
v2: Exporting devcgroup_check_permission() instead of
__devcgroup_check_permission() as per review comments.
Harish Kasiviswanathan (4):
drm/amdkfd: Store kfd_dev in iolink and cache properties
drm/amd: Pass drm_device to kfd
device_cgroup: Export devcgroup_check_permission
drm/amdkfd: Check against device cgroup
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 +
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 2 ++
drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c | 9 ++++++--
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 20 ++++++++++++++++++
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 22 ++++++++++++++++++++
drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 3 +++
include/linux/device_cgroup.h | 19 ++++-------------
security/device_cgroup.c | 15 +++++++++++--
9 files changed, 73 insertions(+), 20 deletions(-)
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread[parent not found: <20190517161435.14121-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>]
* [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission [not found] ` <20190517161435.14121-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> @ 2019-05-17 16:15 ` Kasiviswanathan, Harish [not found] ` <20190517161435.14121-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Kasiviswanathan, Harish @ 2019-05-17 16:15 UTC (permalink / raw) To: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Cc: Kasiviswanathan, Harish For AMD compute (amdkfd) driver. All AMD compute devices are exported via single device node /dev/kfd. As a result devices cannot be controlled individually using device cgroup. AMD compute devices will rely on its graphics counterpart that exposes /dev/dri/renderN node for each device. For each task (based on its cgroup), KFD driver will check if /dev/dri/renderN node is accessible before exposing it. Change-Id: I1b9705b2c30622a27655f4f878980fa138dbf373 Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> --- include/linux/device_cgroup.h | 19 ++++--------------- security/device_cgroup.c | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h index 8557efe096dc..bd19897bd582 100644 --- a/include/linux/device_cgroup.h +++ b/include/linux/device_cgroup.h @@ -12,26 +12,15 @@ #define DEVCG_DEV_ALL 4 /* this represents all devices */ #ifdef CONFIG_CGROUP_DEVICE -extern int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access); +extern int devcgroup_check_permission(short type, u32 major, u32 minor, + short access); #else -static inline int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static inline int devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { return 0; } #endif #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF) -static inline int devcgroup_check_permission(short type, u32 major, u32 minor, - short access) -{ - int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); - - if (rc) - return -EPERM; - - return __devcgroup_check_permission(type, major, minor, access); -} - static inline int devcgroup_inode_permission(struct inode *inode, int mask) { short type, access = 0; diff --git a/security/device_cgroup.c b/security/device_cgroup.c index cd97929fac66..3c57e05bf73b 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -801,8 +801,8 @@ struct cgroup_subsys devices_cgrp_subsys = { * * returns 0 on success, -EPERM case the operation is not permitted */ -int __devcgroup_check_permission(short type, u32 major, u32 minor, - short access) +static int __devcgroup_check_permission(short type, u32 major, u32 minor, + short access) { struct dev_cgroup *dev_cgroup; bool rc; @@ -824,3 +824,14 @@ int __devcgroup_check_permission(short type, u32 major, u32 minor, return 0; } + +int devcgroup_check_permission(short type, u32 major, u32 minor, short access) +{ + int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); + + if (rc) + return -EPERM; + + return __devcgroup_check_permission(type, major, minor, access); +} +EXPORT_SYMBOL(devcgroup_check_permission); -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20190517161435.14121-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission [not found] ` <20190517161435.14121-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org> @ 2019-05-17 17:06 ` Roman Gushchin 0 siblings, 0 replies; 9+ messages in thread From: Roman Gushchin @ 2019-05-17 17:06 UTC (permalink / raw) To: Kasiviswanathan, Harish Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org On Fri, May 17, 2019 at 04:15:06PM +0000, Kasiviswanathan, Harish wrote: > For AMD compute (amdkfd) driver. > > All AMD compute devices are exported via single device node /dev/kfd. As > a result devices cannot be controlled individually using device cgroup. > > AMD compute devices will rely on its graphics counterpart that exposes > /dev/dri/renderN node for each device. For each task (based on its > cgroup), KFD driver will check if /dev/dri/renderN node is accessible > before exposing it. > > Change-Id: I1b9705b2c30622a27655f4f878980fa138dbf373 > Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com> > --- > include/linux/device_cgroup.h | 19 ++++--------------- > security/device_cgroup.c | 15 +++++++++++++-- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h > index 8557efe096dc..bd19897bd582 100644 > --- a/include/linux/device_cgroup.h > +++ b/include/linux/device_cgroup.h > @@ -12,26 +12,15 @@ > #define DEVCG_DEV_ALL 4 /* this represents all devices */ > > #ifdef CONFIG_CGROUP_DEVICE > -extern int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access); > +extern int devcgroup_check_permission(short type, u32 major, u32 minor, > + short access); > #else > -static inline int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > +static inline int devcgroup_check_permission(short type, u32 major, u32 minor, > + short access) > { return 0; } > #endif > > #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF) > -static inline int devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > -{ > - int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); > - > - if (rc) > - return -EPERM; > - > - return __devcgroup_check_permission(type, major, minor, access); > -} > - > static inline int devcgroup_inode_permission(struct inode *inode, int mask) > { > short type, access = 0; > diff --git a/security/device_cgroup.c b/security/device_cgroup.c > index cd97929fac66..3c57e05bf73b 100644 > --- a/security/device_cgroup.c > +++ b/security/device_cgroup.c > @@ -801,8 +801,8 @@ struct cgroup_subsys devices_cgrp_subsys = { > * > * returns 0 on success, -EPERM case the operation is not permitted > */ > -int __devcgroup_check_permission(short type, u32 major, u32 minor, > - short access) > +static int __devcgroup_check_permission(short type, u32 major, u32 minor, > + short access) > { > struct dev_cgroup *dev_cgroup; > bool rc; > @@ -824,3 +824,14 @@ int __devcgroup_check_permission(short type, u32 major, u32 minor, > > return 0; > } > + > +int devcgroup_check_permission(short type, u32 major, u32 minor, short access) > +{ > + int rc = BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type, major, minor, access); > + > + if (rc) > + return -EPERM; > + > + return __devcgroup_check_permission(type, major, minor, access); > +} > +EXPORT_SYMBOL(devcgroup_check_permission); Perfect, now looks good to me! Please, feel free to use my acks for patches 3 and 4: Acked-by: Roman Gushchin <guro@fb.com> Thanks! _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-09-24 17:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-16 18:05 AMDKFD (AMD GPU compute) support for device cgroup Kasiviswanathan, Harish
[not found] ` <20190916180523.27341-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
2019-09-16 18:05 ` [PATCH v2 1/4] drm/amdkfd: Store kfd_dev in iolink and cache properties Kasiviswanathan, Harish
2019-09-16 18:05 ` [PATCH v2 2/4] drm/amd: Pass drm_device to kfd Kasiviswanathan, Harish
2019-09-16 18:05 ` [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission Kasiviswanathan, Harish
[not found] ` <20190916180523.27341-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
2019-09-24 15:54 ` Kasiviswanathan, Harish
[not found] ` <MN2PR12MB2911F59E9B91AAD349B4E40F8C840-rweVpJHSKTpv4wpD+z6awQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-09-24 17:13 ` Roman Gushchin
2019-09-16 18:05 ` [PATCH v2 4/4] drm/amdkfd: Check against device cgroup Kasiviswanathan, Harish
-- strict thread matches above, loose matches on Subject: below --
2019-05-17 16:14 [PATCH v2 0/4] AMDKFD (AMD GPU compute) support for " Kasiviswanathan, Harish
[not found] ` <20190517161435.14121-1-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
2019-05-17 16:15 ` [PATCH v2 3/4] device_cgroup: Export devcgroup_check_permission Kasiviswanathan, Harish
[not found] ` <20190517161435.14121-4-Harish.Kasiviswanathan-5C7GfCeVMHo@public.gmane.org>
2019-05-17 17:06 ` Roman Gushchin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox