[PATCH AUTOSEL 4.14 4/7] drm: Copy drm_wait_vblank to user before returning

[PATCH AUTOSEL 4.14 4/7] drm: Copy drm_wait_vblank to user before returning

[Sasha Levin]

From: Mark Yacoub <markyacoub@google.com>

[ Upstream commit fa0b1ef5f7a694f48e00804a391245f3471aa155 ]

[Why]
Userspace should get back a copy of drm_wait_vblank that's been modified
even when drm_wait_vblank_ioctl returns a failure.

Rationale:
drm_wait_vblank_ioctl modifies the request and expects the user to read
it back. When the type is RELATIVE, it modifies it to ABSOLUTE and updates
the sequence to become current_vblank_count + sequence (which was
RELATIVE), but now it became ABSOLUTE.
drmWaitVBlank (in libdrm) expects this to be the case as it modifies
the request to be Absolute so it expects the sequence to would have been
updated.

The change is in compat_drm_wait_vblank, which is called by
drm_compat_ioctl. This change of copying the data back regardless of the
return number makes it en par with drm_ioctl, which always copies the
data before returning.

[How]
Return from the function after everything has been copied to user.

Fixes IGT:kms_flip::modeset-vs-vblank-race-interruptible
Tested on ChromeOS Trogdor(msm)

Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210812194917.1703356-1-markyacoub@chromium.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/drm_ioc32.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index 86105e7f07fc..6773e8f09fc3 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -855,8 +855,6 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
 	req.request.sequence = req32.request.sequence;
 	req.request.signal = req32.request.signal;
 	err = drm_ioctl_kernel(file, drm_wait_vblank_ioctl, &req, DRM_UNLOCKED);
-	if (err)
-		return err;
 
 	req32.reply.type = req.reply.type;
 	req32.reply.sequence = req.reply.sequence;
@@ -865,7 +863,7 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
 	if (copy_to_user(argp, &req32, sizeof(req32)))
 		return -EFAULT;
 
-	return 0;
+	return err;
 }
 
 #if defined(CONFIG_X86)
-- 
2.30.2

[PATCH AUTOSEL 4.14 5/7] drm/nouveau/disp: power down unused DP links during init

[Sasha Levin]

From: Ben Skeggs <bskeggs@redhat.com>

[ Upstream commit 6eaa1f3c59a707332e921e32782ffcad49915c5e ]

When booted with multiple displays attached, the EFI GOP driver on (at
least) Ampere, can leave DP links powered up that aren't being used to
display anything.  This confuses our tracking of SOR routing, with the
likely result being a failed modeset and display engine hang.

Fix this by (ab?)using the DisableLT IED script to power-down the link,
restoring HW to a state the driver expects.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c   | 2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.h   | 1 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c | 9 +++++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
index 5e51a5c1eb01..d11cb1f887f7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
@@ -418,7 +418,7 @@ nvkm_dp_train(struct nvkm_dp *dp, u32 dataKBps)
 	return ret;
 }
 
-static void
+void
 nvkm_dp_disable(struct nvkm_outp *outp, struct nvkm_ior *ior)
 {
 	struct nvkm_dp *dp = nvkm_dp(outp);
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.h b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.h
index 495f665a0ee6..12d6ff4cfa95 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.h
@@ -32,6 +32,7 @@ struct nvkm_dp {
 
 int nvkm_dp_new(struct nvkm_disp *, int index, struct dcb_output *,
 		struct nvkm_outp **);
+void nvkm_dp_disable(struct nvkm_outp *, struct nvkm_ior *);
 
 /* DPCD Receiver Capabilities */
 #define DPCD_RC00_DPCD_REV                                              0x00000
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
index bbba77ff9385..81c0f0513c74 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
@@ -22,6 +22,7 @@
  * Authors: Ben Skeggs
  */
 #include "outp.h"
+#include "dp.h"
 #include "ior.h"
 
 #include <subdev/bios.h>
@@ -207,6 +208,14 @@ nvkm_outp_init_route(struct nvkm_outp *outp)
 	if (!ior->arm.head || ior->arm.proto != proto) {
 		OUTP_DBG(outp, "no heads (%x %d %d)", ior->arm.head,
 			 ior->arm.proto, proto);
+
+		/* The EFI GOP driver on Ampere can leave unused DP links routed,
+		 * which we don't expect.  The DisableLT IED script *should* get
+		 * us back to where we need to be.
+		 */
+		if (ior->func->route.get && !ior->arm.head && outp->info.type == DCB_OUTPUT_DP)
+			nvkm_dp_disable(outp, ior);
+
 		return;
 	}
 
-- 
2.30.2

[PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace

[Sasha Levin]

From: Ben Skeggs <bskeggs@redhat.com>

[ Upstream commit 148a8653789c01f159764ffcc3f370008966b42f ]

Long ago, there had been plans for making use of a bunch of these APIs
from userspace and there's various checks in place to stop misbehaving.

Countless other projects have occurred in the meantime, and the pieces
didn't finish falling into place for that to happen.

They will (hopefully) in the not-too-distant future, but it won't look
quite as insane.  The super checks are causing problems right now, and
are going to be removed.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/nouveau/include/nvif/cl0080.h |  3 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  1 +
 drivers/gpu/drm/nouveau/nouveau_usif.c        | 57 ++++++++++++++-----
 .../gpu/drm/nouveau/nvkm/engine/device/user.c |  2 +-
 4 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
index 2740278d226b..61c17acd507c 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
@@ -4,7 +4,8 @@
 
 struct nv_device_v0 {
 	__u8  version;
-	__u8  pad01[7];
+	__u8  priv;
+	__u8  pad02[6];
 	__u64 device;	/* device identifier, ~0 for client default */
 };
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index fb6b1d0f7fef..fc54a26598cc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -151,6 +151,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char *sname,
 	ret = nvif_device_init(&cli->base.object, 0, NV_DEVICE,
 			       &(struct nv_device_v0) {
 					.device = ~0,
+					.priv = true,
 			       }, sizeof(struct nv_device_v0),
 			       &cli->device);
 	if (ret) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_usif.c b/drivers/gpu/drm/nouveau/nouveau_usif.c
index 9dc10b17ad34..5da1f4d223d7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_usif.c
+++ b/drivers/gpu/drm/nouveau/nouveau_usif.c
@@ -32,6 +32,9 @@
 #include <nvif/event.h>
 #include <nvif/ioctl.h>
 
+#include <nvif/class.h>
+#include <nvif/cl0080.h>
+
 struct usif_notify_p {
 	struct drm_pending_event base;
 	struct {
@@ -261,7 +264,7 @@ usif_object_dtor(struct usif_object *object)
 }
 
 static int
-usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32 argc)
+usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32 argc, bool parent_abi16)
 {
 	struct nouveau_cli *cli = nouveau_cli(f);
 	struct nvif_client *client = &cli->base;
@@ -271,23 +274,48 @@ usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32 argc)
 	struct usif_object *object;
 	int ret = -ENOSYS;
 
+	if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true)))
+		return ret;
+
+	switch (args->v0.oclass) {
+	case NV_DMA_FROM_MEMORY:
+	case NV_DMA_TO_MEMORY:
+	case NV_DMA_IN_MEMORY:
+		return -EINVAL;
+	case NV_DEVICE: {
+		union {
+			struct nv_device_v0 v0;
+		} *args = data;
+
+		if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, false)))
+			return ret;
+
+		args->v0.priv = false;
+		break;
+	}
+	default:
+		if (!parent_abi16)
+			return -EINVAL;
+		break;
+	}
+
 	if (!(object = kmalloc(sizeof(*object), GFP_KERNEL)))
 		return -ENOMEM;
 	list_add(&object->head, &cli->objects);
 
-	if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true))) {
-		object->route = args->v0.route;
-		object->token = args->v0.token;
-		args->v0.route = NVDRM_OBJECT_USIF;
-		args->v0.token = (unsigned long)(void *)object;
-		ret = nvif_client_ioctl(client, argv, argc);
-		args->v0.token = object->token;
-		args->v0.route = object->route;
+	object->route = args->v0.route;
+	object->token = args->v0.token;
+	args->v0.route = NVDRM_OBJECT_USIF;
+	args->v0.token = (unsigned long)(void *)object;
+	ret = nvif_client_ioctl(client, argv, argc);
+	if (ret) {
+		usif_object_dtor(object);
+		return ret;
 	}
 
-	if (ret)
-		usif_object_dtor(object);
-	return ret;
+	args->v0.token = object->token;
+	args->v0.route = object->route;
+	return 0;
 }
 
 int
@@ -301,6 +329,7 @@ usif_ioctl(struct drm_file *filp, void __user *user, u32 argc)
 		struct nvif_ioctl_v0 v0;
 	} *argv = data;
 	struct usif_object *object;
+	bool abi16 = false;
 	u8 owner;
 	int ret;
 
@@ -331,11 +360,13 @@ usif_ioctl(struct drm_file *filp, void __user *user, u32 argc)
 			mutex_unlock(&cli->mutex);
 			goto done;
 		}
+
+		abi16 = true;
 	}
 
 	switch (argv->v0.type) {
 	case NVIF_IOCTL_V0_NEW:
-		ret = usif_object_new(filp, data, size, argv, argc);
+		ret = usif_object_new(filp, data, size, argv, argc, abi16);
 		break;
 	case NVIF_IOCTL_V0_NTFY_NEW:
 		ret = usif_notify_new(filp, data, size, argv, argc);
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
index 513ee6b79553..08100eed9584 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
@@ -347,7 +347,7 @@ nvkm_udevice_new(const struct nvkm_oclass *oclass, void *data, u32 size,
 		return ret;
 
 	/* give priviledged clients register access */
-	if (client->super)
+	if (args->v0.priv)
 		func = &nvkm_udevice_super;
 	else
 		func = &nvkm_udevice;
-- 
2.30.2

Re: [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace

[Lyude Paul]

This isn't at all intended to be a fix to be backported, so I don't think this
should be included. I don't know about 5/7, but I'll let Benjamin comment on
that one

On Mon, 2021-08-23 at 20:55 -0400, Sasha Levin wrote:
> From: Ben Skeggs <bskeggs@redhat.com>
> 
> [ Upstream commit 148a8653789c01f159764ffcc3f370008966b42f ]
> 
> Long ago, there had been plans for making use of a bunch of these APIs
> from userspace and there's various checks in place to stop misbehaving.
> 
> Countless other projects have occurred in the meantime, and the pieces
> didn't finish falling into place for that to happen.
> 
> They will (hopefully) in the not-too-distant future, but it won't look
> quite as insane.  The super checks are causing problems right now, and
> are going to be removed.
> 
> Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/gpu/drm/nouveau/include/nvif/cl0080.h |  3 +-
>  drivers/gpu/drm/nouveau/nouveau_drm.c         |  1 +
>  drivers/gpu/drm/nouveau/nouveau_usif.c        | 57 ++++++++++++++-----
>  .../gpu/drm/nouveau/nvkm/engine/device/user.c |  2 +-
>  4 files changed, 48 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> index 2740278d226b..61c17acd507c 100644
> --- a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> +++ b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> @@ -4,7 +4,8 @@
>  
>  struct nv_device_v0 {
>         __u8  version;
> -       __u8  pad01[7];
> +       __u8  priv;
> +       __u8  pad02[6];
>         __u64 device;   /* device identifier, ~0 for client default */
>  };
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index fb6b1d0f7fef..fc54a26598cc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -151,6 +151,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char
> *sname,
>         ret = nvif_device_init(&cli->base.object, 0, NV_DEVICE,
>                                &(struct nv_device_v0) {
>                                         .device = ~0,
> +                                       .priv = true,
>                                }, sizeof(struct nv_device_v0),
>                                &cli->device);
>         if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_usif.c
> b/drivers/gpu/drm/nouveau/nouveau_usif.c
> index 9dc10b17ad34..5da1f4d223d7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_usif.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_usif.c
> @@ -32,6 +32,9 @@
>  #include <nvif/event.h>
>  #include <nvif/ioctl.h>
>  
> +#include <nvif/class.h>
> +#include <nvif/cl0080.h>
> +
>  struct usif_notify_p {
>         struct drm_pending_event base;
>         struct {
> @@ -261,7 +264,7 @@ usif_object_dtor(struct usif_object *object)
>  }
>  
>  static int
> -usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32
> argc)
> +usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32
> argc, bool parent_abi16)
>  {
>         struct nouveau_cli *cli = nouveau_cli(f);
>         struct nvif_client *client = &cli->base;
> @@ -271,23 +274,48 @@ usif_object_new(struct drm_file *f, void *data, u32
> size, void *argv, u32 argc)
>         struct usif_object *object;
>         int ret = -ENOSYS;
>  
> +       if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true)))
> +               return ret;
> +
> +       switch (args->v0.oclass) {
> +       case NV_DMA_FROM_MEMORY:
> +       case NV_DMA_TO_MEMORY:
> +       case NV_DMA_IN_MEMORY:
> +               return -EINVAL;
> +       case NV_DEVICE: {
> +               union {
> +                       struct nv_device_v0 v0;
> +               } *args = data;
> +
> +               if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0,
> false)))
> +                       return ret;
> +
> +               args->v0.priv = false;
> +               break;
> +       }
> +       default:
> +               if (!parent_abi16)
> +                       return -EINVAL;
> +               break;
> +       }
> +
>         if (!(object = kmalloc(sizeof(*object), GFP_KERNEL)))
>                 return -ENOMEM;
>         list_add(&object->head, &cli->objects);
>  
> -       if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true))) {
> -               object->route = args->v0.route;
> -               object->token = args->v0.token;
> -               args->v0.route = NVDRM_OBJECT_USIF;
> -               args->v0.token = (unsigned long)(void *)object;
> -               ret = nvif_client_ioctl(client, argv, argc);
> -               args->v0.token = object->token;
> -               args->v0.route = object->route;
> +       object->route = args->v0.route;
> +       object->token = args->v0.token;
> +       args->v0.route = NVDRM_OBJECT_USIF;
> +       args->v0.token = (unsigned long)(void *)object;
> +       ret = nvif_client_ioctl(client, argv, argc);
> +       if (ret) {
> +               usif_object_dtor(object);
> +               return ret;
>         }
>  
> -       if (ret)
> -               usif_object_dtor(object);
> -       return ret;
> +       args->v0.token = object->token;
> +       args->v0.route = object->route;
> +       return 0;
>  }
>  
>  int
> @@ -301,6 +329,7 @@ usif_ioctl(struct drm_file *filp, void __user *user, u32
> argc)
>                 struct nvif_ioctl_v0 v0;
>         } *argv = data;
>         struct usif_object *object;
> +       bool abi16 = false;
>         u8 owner;
>         int ret;
>  
> @@ -331,11 +360,13 @@ usif_ioctl(struct drm_file *filp, void __user *user,
> u32 argc)
>                         mutex_unlock(&cli->mutex);
>                         goto done;
>                 }
> +
> +               abi16 = true;
>         }
>  
>         switch (argv->v0.type) {
>         case NVIF_IOCTL_V0_NEW:
> -               ret = usif_object_new(filp, data, size, argv, argc);
> +               ret = usif_object_new(filp, data, size, argv, argc, abi16);
>                 break;
>         case NVIF_IOCTL_V0_NTFY_NEW:
>                 ret = usif_notify_new(filp, data, size, argv, argc);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> index 513ee6b79553..08100eed9584 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> @@ -347,7 +347,7 @@ nvkm_udevice_new(const struct nvkm_oclass *oclass, void
> *data, u32 size,
>                 return ret;
>  
>         /* give priviledged clients register access */
> -       if (client->super)
> +       if (args->v0.priv)
>                 func = &nvkm_udevice_super;
>         else
>                 func = &nvkm_udevice;

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace

[Sasha Levin]

On Tue, Aug 24, 2021 at 01:05:32PM -0400, Lyude Paul wrote:
>This isn't at all intended to be a fix to be backported, so I don't think this
>should be included. I don't know about 5/7, but I'll let Benjamin comment on
>that one

I'll drop it, thanks!

-- 
Thanks,
Sasha