Karol Herbst
2024-Mar-04 18:31 UTC
[PATCH 1/2] drm/nouveau: fix stale locked mutex in nouveau_gem_ioctl_pushbuf
If VM_BIND is enabled on the client the legacy submission ioctl can't be used, however if a client tries to do so regardless it will return an error. In this case the clients mutex remained unlocked leading to a deadlock inside nouveau_drm_postclose or any other nouveau ioctl call. Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI") Cc: Danilo Krummrich <dakr at redhat.com> Signed-off-by: Karol Herbst <kherbst at redhat.com> --- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 49c2bcbef1299..5a887d67dc0e8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -764,7 +764,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, return -ENOMEM; if (unlikely(nouveau_cli_uvmm(cli))) - return -ENOSYS; + return nouveau_abi16_put(abi16, -ENOSYS); list_for_each_entry(temp, &abi16->channels, head) { if (temp->chan->chid == req->channel) { -- 2.43.2
Those are already de-facto UAPI, so let's just move it into the uapi header. Signed-off-by: Karol Herbst <kherbst at redhat.com> --- drivers/gpu/drm/nouveau/nouveau_abi16.c | 20 +++++++++++++++----- drivers/gpu/drm/nouveau/nouveau_abi16.h | 12 ------------ include/uapi/drm/nouveau_drm.h | 22 ++++++++++++++++++++++ 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c index cd14f993bdd1b..92f9127b284ac 100644 --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c @@ -312,11 +312,21 @@ nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS) if (device->info.family >= NV_DEVICE_INFO_V0_KEPLER) { if (init->fb_ctxdma_handle == ~0) { switch (init->tt_ctxdma_handle) { - case 0x01: engine = NV_DEVICE_HOST_RUNLIST_ENGINES_GR ; break; - case 0x02: engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSPDEC; break; - case 0x04: engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSPPP ; break; - case 0x08: engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSVLD ; break; - case 0x30: engine = NV_DEVICE_HOST_RUNLIST_ENGINES_CE ; break; + case NOUVEAU_FIFO_ENGINE_GR: + engine = NV_DEVICE_HOST_RUNLIST_ENGINES_GR; + break; + case NOUVEAU_FIFO_ENGINE_VP: + engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSPDEC; + break; + case NOUVEAU_FIFO_ENGINE_PPP: + engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSPPP; + break; + case NOUVEAU_FIFO_ENGINE_BSP: + engine = NV_DEVICE_HOST_RUNLIST_ENGINES_MSVLD; + break; + case NOUVEAU_FIFO_ENGINE_CE: + engine = NV_DEVICE_HOST_RUNLIST_ENGINES_CE; + break; default: return nouveau_abi16_put(abi16, -ENOSYS); } diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h index 11c8c4a80079b..661b901d8ecc9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_abi16.h +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h @@ -50,18 +50,6 @@ struct drm_nouveau_grobj_alloc { int class; }; -struct drm_nouveau_notifierobj_alloc { - uint32_t channel; - uint32_t handle; - uint32_t size; - uint32_t offset; -}; - -struct drm_nouveau_gpuobj_free { - int channel; - uint32_t handle; -}; - struct drm_nouveau_setparam { uint64_t param; uint64_t value; diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h index 77d7ff0d5b110..5404d4cfff4c2 100644 --- a/include/uapi/drm/nouveau_drm.h +++ b/include/uapi/drm/nouveau_drm.h @@ -73,6 +73,16 @@ struct drm_nouveau_getparam { __u64 value; }; +/* + * Those are used to support selecting the main engine used on Kepler. + * This goes into drm_nouveau_channel_alloc::tt_ctxdma_handle + */ +#define NOUVEAU_FIFO_ENGINE_GR 0x01 +#define NOUVEAU_FIFO_ENGINE_VP 0x02 +#define NOUVEAU_FIFO_ENGINE_PPP 0x04 +#define NOUVEAU_FIFO_ENGINE_BSP 0x08 +#define NOUVEAU_FIFO_ENGINE_CE 0x30 + struct drm_nouveau_channel_alloc { __u32 fb_ctxdma_handle; __u32 tt_ctxdma_handle; @@ -95,6 +105,18 @@ struct drm_nouveau_channel_free { __s32 channel; }; +struct drm_nouveau_notifierobj_alloc { + __u32 channel; + __u32 handle; + __u32 size; + __u32 offset; +}; + +struct drm_nouveau_gpuobj_free { + __s32 channel; + __u32 handle; +}; + #define NOUVEAU_GEM_DOMAIN_CPU (1 << 0) #define NOUVEAU_GEM_DOMAIN_VRAM (1 << 1) #define NOUVEAU_GEM_DOMAIN_GART (1 << 2) -- 2.43.2
Lyude Paul
2024-Mar-04 19:30 UTC
[PATCH 1/2] drm/nouveau: fix stale locked mutex in nouveau_gem_ioctl_pushbuf
Reviewed-by: Lyude Paul <lyude at redhat.com> On Mon, 2024-03-04 at 19:31 +0100, Karol Herbst wrote:> If VM_BIND is enabled on the client the legacy submission ioctl can't be > used, however if a client tries to do so regardless it will return an > error. In this case the clients mutex remained unlocked leading to a > deadlock inside nouveau_drm_postclose or any other nouveau ioctl call. > > Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI") > Cc: Danilo Krummrich <dakr at redhat.com> > Signed-off-by: Karol Herbst <kherbst at redhat.com> > --- > drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c > index 49c2bcbef1299..5a887d67dc0e8 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c > @@ -764,7 +764,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, > return -ENOMEM; > > if (unlikely(nouveau_cli_uvmm(cli))) > - return -ENOSYS; > + return nouveau_abi16_put(abi16, -ENOSYS); > > list_for_each_entry(temp, &abi16->channels, head) { > if (temp->chan->chid == req->channel) {-- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Danilo Krummrich
2024-Mar-05 09:18 UTC
[PATCH 1/2] drm/nouveau: fix stale locked mutex in nouveau_gem_ioctl_pushbuf
On 3/4/24 19:31, Karol Herbst wrote:> If VM_BIND is enabled on the client the legacy submission ioctl can't be > used, however if a client tries to do so regardless it will return an > error. In this case the clients mutex remained unlocked leading to a > deadlock inside nouveau_drm_postclose or any other nouveau ioctl call. > > Fixes: b88baab82871 ("drm/nouveau: implement new VM_BIND uAPI") > Cc: Danilo Krummrich <dakr at redhat.com> > Signed-off-by: Karol Herbst <kherbst at redhat.com>Should add a stable tag for that one, otherwise: Reviewed-by: Danilo Krummrich <dakr at redhat.com>> --- > drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c > index 49c2bcbef1299..5a887d67dc0e8 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c > @@ -764,7 +764,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, > return -ENOMEM; > > if (unlikely(nouveau_cli_uvmm(cli))) > - return -ENOSYS; > + return nouveau_abi16_put(abi16, -ENOSYS); > > list_for_each_entry(temp, &abi16->channels, head) { > if (temp->chan->chid == req->channel) {