All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@redhat.com>
To: Ben Skeggs <bskeggs@nvidia.com>
Cc: nouveau@lists.freedesktop.org
Subject: Re: [PATCH v2 15/37] drm/nouveau/nvif: remove route/token
Date: Fri, 19 Jul 2024 14:12:42 +0200	[thread overview]
Message-ID: <ZppYOmWfup8lwC3L@pollux> (raw)
In-Reply-To: <839e9000-aa4e-4879-aa0d-6c17fd19699c@nvidia.com>

On Thu, Jul 18, 2024 at 05:52:44PM +1000, Ben Skeggs wrote:
> On 10/7/24 02:11, Danilo Krummrich wrote:
> 
> > On Fri, Jul 05, 2024 at 04:36:59AM +1000, Ben Skeggs wrote:
> > > These were a cludge used to prevent userspace's nvif ioctl from
> > > accessing objects created by the kernel for the same client.
> > > 
> > > That interface was removed in a previous patch, so these are no
> > > longer useful for anything.
> > It would probably be good to move this patch directly after the one it depends
> > on.
> 
> Is this really necessary?  The ordering of this series is already incredibly
> fragile, with a bunch of chicken/egg situations that were tough to
> untangle.  Moreover, I don't see what real value it adds at this point.

Not for this series, but in general. If such large series are required, having
directly related ones close to each other, makes it way easier to review.

> 
> > 
> > > Signed-off-by: Ben Skeggs <bskeggs@nvidia.com>
> > > ---
> > >   drivers/gpu/drm/nouveau/include/nvif/client.h |  1 -
> > >   .../drm/nouveau/include/nvkm/core/object.h    |  2 --
> > >   .../drm/nouveau/include/nvkm/core/oclass.h    |  2 --
> > >   drivers/gpu/drm/nouveau/nouveau_abi16.c       |  8 --------
> > >   drivers/gpu/drm/nouveau/nvif/client.c         |  1 -
> > >   drivers/gpu/drm/nouveau/nvif/object.c         |  3 ---
> > >   drivers/gpu/drm/nouveau/nvkm/core/client.c    |  2 --
> > >   drivers/gpu/drm/nouveau/nvkm/core/ioctl.c     | 19 ++++---------------
> > >   drivers/gpu/drm/nouveau/nvkm/core/object.c    |  2 --
> > >   drivers/gpu/drm/nouveau/nvkm/core/uevent.c    |  4 ++--
> > >   10 files changed, 6 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/include/nvif/client.h b/drivers/gpu/drm/nouveau/include/nvif/client.h
> > > index 5d9395e651b6..64b033222c56 100644
> > > --- a/drivers/gpu/drm/nouveau/include/nvif/client.h
> > > +++ b/drivers/gpu/drm/nouveau/include/nvif/client.h
> > > @@ -8,7 +8,6 @@ struct nvif_client {
> > >   	struct nvif_object object;
> > >   	const struct nvif_driver *driver;
> > >   	u64 version;
> > > -	u8 route;
> > >   };
> > >   int  nvif_client_ctor(struct nvif_client *parent, const char *name, u64 device,
> > > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/object.h b/drivers/gpu/drm/nouveau/include/nvkm/core/object.h
> > > index ed1f66360782..d4f1c964ba31 100644
> > > --- a/drivers/gpu/drm/nouveau/include/nvkm/core/object.h
> > > +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/object.h
> > > @@ -15,8 +15,6 @@ struct nvkm_object {
> > >   	struct list_head head;
> > >   	struct list_head tree;
> > > -	u8  route;
> > > -	u64 token;
> > >   	u64 object;
> > >   	struct rb_node node;
> > >   };
> > > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h b/drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h
> > > index 8e1b945d38f3..cad05f0e7948 100644
> > > --- a/drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h
> > > +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h
> > > @@ -21,8 +21,6 @@ struct nvkm_oclass {
> > >   	const void *priv;
> > >   	const void *engn;
> > >   	u32 handle;
> > > -	u8  route;
> > > -	u64 token;
> > >   	u64 object;
> > >   	struct nvkm_client *client;
> > >   	struct nvkm_object *parent;
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > > index 6f0548e57f9e..704977530b6b 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > > @@ -521,7 +521,6 @@ nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS)
> > >   	struct nouveau_abi16 *abi16 = nouveau_abi16_get(file_priv);
> > >   	struct nouveau_abi16_chan *chan;
> > >   	struct nouveau_abi16_ntfy *ntfy;
> > > -	struct nvif_client *client;
> > >   	struct nvif_sclass *sclass;
> > >   	s32 oclass = 0;
> > >   	int ret, i;
> > > @@ -531,7 +530,6 @@ nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS)
> > >   	if (init->handle == ~0)
> > >   		return nouveau_abi16_put(abi16, -EINVAL);
> > > -	client = &abi16->cli->base;
> > >   	chan = nouveau_abi16_chan(abi16, init->channel);
> > >   	if (!chan)
> > > @@ -596,10 +594,8 @@ nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS)
> > >   	list_add(&ntfy->head, &chan->notifiers);
> > > -	client->route = NVDRM_OBJECT_ABI16;
> > >   	ret = nvif_object_ctor(&chan->chan->user, "abi16EngObj", init->handle,
> > >   			       oclass, NULL, 0, &ntfy->object);
> > > -	client->route = NVDRM_OBJECT_NVIF;
> > >   	if (ret)
> > >   		nouveau_abi16_ntfy_fini(chan, ntfy);
> > > @@ -615,7 +611,6 @@ nouveau_abi16_ioctl_notifierobj_alloc(ABI16_IOCTL_ARGS)
> > >   	struct nouveau_abi16_chan *chan;
> > >   	struct nouveau_abi16_ntfy *ntfy;
> > >   	struct nvif_device *device;
> > > -	struct nvif_client *client;
> > >   	struct nv_dma_v0 args = {};
> > >   	int ret;
> > > @@ -626,7 +621,6 @@ nouveau_abi16_ioctl_notifierobj_alloc(ABI16_IOCTL_ARGS)
> > >   	/* completely unnecessary for these chipsets... */
> > >   	if (unlikely(device->info.family >= NV_DEVICE_INFO_V0_FERMI))
> > >   		return nouveau_abi16_put(abi16, -EINVAL);
> > > -	client = &abi16->cli->base;
> > >   	chan = nouveau_abi16_chan(abi16, info->channel);
> > >   	if (!chan)
> > > @@ -663,11 +657,9 @@ nouveau_abi16_ioctl_notifierobj_alloc(ABI16_IOCTL_ARGS)
> > >   		args.limit += chan->ntfy->offset;
> > >   	}
> > > -	client->route = NVDRM_OBJECT_ABI16;
> > >   	ret = nvif_object_ctor(&chan->chan->user, "abi16Ntfy", info->handle,
> > >   			       NV_DMA_IN_MEMORY, &args, sizeof(args),
> > >   			       &ntfy->object);
> > > -	client->route = NVDRM_OBJECT_NVIF;
> > >   	if (ret)
> > >   		goto done;
> > > diff --git a/drivers/gpu/drm/nouveau/nvif/client.c b/drivers/gpu/drm/nouveau/nvif/client.c
> > > index 3a27245f467f..cd5439b73ac7 100644
> > > --- a/drivers/gpu/drm/nouveau/nvif/client.c
> > > +++ b/drivers/gpu/drm/nouveau/nvif/client.c
> > > @@ -79,7 +79,6 @@ nvif_client_ctor(struct nvif_client *parent, const char *name, u64 device,
> > >   	client->object.client = client;
> > >   	client->object.handle = ~0;
> > > -	client->route = NVIF_IOCTL_V0_ROUTE_NVIF;
> > >   	client->driver = parent->driver;
> > >   	if (ret == 0) {
> > > diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c
> > > index 4d1aaee8fe15..2b3e05197846 100644
> > > --- a/drivers/gpu/drm/nouveau/nvif/object.c
> > > +++ b/drivers/gpu/drm/nouveau/nvif/object.c
> > > @@ -40,7 +40,6 @@ nvif_object_ioctl(struct nvif_object *object, void *data, u32 size, void **hack)
> > >   			args->v0.object = nvif_handle(object);
> > >   		else
> > >   			args->v0.object = 0;
> > > -		args->v0.owner = NVIF_IOCTL_V0_OWNER_ANY;
> > >   	} else
> > >   		return -ENOSYS;
> > > @@ -286,8 +285,6 @@ nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle,
> > >   		args->ioctl.version = 0;
> > >   		args->ioctl.type = NVIF_IOCTL_V0_NEW;
> > >   		args->new.version = 0;
> > > -		args->new.route = parent->client->route;
> > > -		args->new.token = nvif_handle(object);
> > >   		args->new.object = nvif_handle(object);
> > >   		args->new.handle = handle;
> > >   		args->new.oclass = oclass;
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/client.c b/drivers/gpu/drm/nouveau/nvkm/core/client.c
> > > index 48416c5039a1..95cbb5b682f2 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/core/client.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/core/client.c
> > > @@ -51,8 +51,6 @@ nvkm_uclient_new(const struct nvkm_oclass *oclass, void *argv, u32 argc,
> > >   	client->object.client = oclass->client;
> > >   	client->object.handle = oclass->handle;
> > > -	client->object.route  = oclass->route;
> > > -	client->object.token  = oclass->token;
> > >   	client->object.object = oclass->object;
> > >   	client->debug = oclass->client->debug;
> > >   	*pobject = &client->object;
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > > index 0b33287e43a7..39d5c9700867 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c
> > > @@ -112,10 +112,9 @@ nvkm_ioctl_new(struct nvkm_client *client,
> > >   	nvif_ioctl(parent, "new size %d\n", size);
> > >   	if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true))) {
> > > -		nvif_ioctl(parent, "new vers %d handle %08x class %08x "
> > > -				   "route %02x token %llx object %016llx\n",
> > > +		nvif_ioctl(parent, "new vers %d handle %08x class %08x object %016llx\n",
> > >   			   args->v0.version, args->v0.handle, args->v0.oclass,
> > > -			   args->v0.route, args->v0.token, args->v0.object);
> > > +			   args->v0.object);
> > >   	} else
> > >   		return ret;
> > > @@ -127,8 +126,6 @@ nvkm_ioctl_new(struct nvkm_client *client,
> > >   	do {
> > >   		memset(&oclass, 0x00, sizeof(oclass));
> > >   		oclass.handle = args->v0.handle;
> > > -		oclass.route  = args->v0.route;
> > > -		oclass.token  = args->v0.token;
> > >   		oclass.object = args->v0.object;
> > >   		oclass.client = client;
> > >   		oclass.parent = parent;
> > > @@ -331,7 +328,7 @@ nvkm_ioctl_v0[] = {
> > >   static int
> > >   nvkm_ioctl_path(struct nvkm_client *client, u64 handle, u32 type,
> > > -		void *data, u32 size, u8 owner, u8 *route, u64 *token)
> > > +		void *data, u32 size)
> > >   {
> > >   	struct nvkm_object *object;
> > >   	int ret;
> > > @@ -342,13 +339,6 @@ nvkm_ioctl_path(struct nvkm_client *client, u64 handle, u32 type,
> > >   		return PTR_ERR(object);
> > >   	}
> > > -	if (owner != NVIF_IOCTL_V0_OWNER_ANY && owner != object->route) {
> > > -		nvif_ioctl(&client->object, "route != owner\n");
> > > -		return -EACCES;
> > > -	}
> > > -	*route = object->route;
> > > -	*token = object->token;
> > > -
> > >   	if (ret = -EINVAL, type < ARRAY_SIZE(nvkm_ioctl_v0)) {
> > >   		if (nvkm_ioctl_v0[type].version == 0)
> > >   			ret = nvkm_ioctl_v0[type].func(client, object, data, size);
> > > @@ -374,8 +364,7 @@ nvkm_ioctl(struct nvkm_client *client, void *data, u32 size, void **hack)
> > >   			   args->v0.version, args->v0.type, args->v0.object,
> > >   			   args->v0.owner);
> > >   		ret = nvkm_ioctl_path(client, args->v0.object, args->v0.type,
> > > -				      data, size, args->v0.owner,
> > > -				      &args->v0.route, &args->v0.token);
> > > +				      data, size);
> > >   	}
> > >   	if (ret != 1) {
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/object.c b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> > > index aea3ba72027a..580b8c7f25af 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/core/object.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> > > @@ -313,8 +313,6 @@ nvkm_object_ctor(const struct nvkm_object_func *func,
> > >   	object->engine = nvkm_engine_ref(oclass->engine);
> > >   	object->oclass = oclass->base.oclass;
> > >   	object->handle = oclass->handle;
> > > -	object->route  = oclass->route;
> > > -	object->token  = oclass->token;
> > >   	object->object = oclass->object;
> > >   	INIT_LIST_HEAD(&object->head);
> > >   	INIT_LIST_HEAD(&object->tree);
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/uevent.c b/drivers/gpu/drm/nouveau/nvkm/core/uevent.c
> > > index ba9d9edaec75..cc254c390a57 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/core/uevent.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/core/uevent.c
> > > @@ -116,9 +116,9 @@ nvkm_uevent_ntfy(struct nvkm_event_ntfy *ntfy, u32 bits)
> > >   	struct nvkm_client *client = uevent->object.client;
> > >   	if (uevent->func)
> > > -		return uevent->func(uevent->parent, uevent->object.token, bits);
> > > +		return uevent->func(uevent->parent, uevent->object.object, bits);
> > > -	return client->event(uevent->object.token, NULL, 0);
> > > +	return client->event(uevent->object.object, NULL, 0);
> > >   }
> > >   int
> > > -- 
> > > 2.45.1
> > > 
> 


  reply	other threads:[~2024-07-19 12:12 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-04 18:36 [PATCH v2 00/37] drm/nouveau: misc. cleanups and removal of unused apis Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 01/37] drm/nouveau: move nouveau_drm_device_fini() above init() Ben Skeggs
2024-07-09 14:48   ` Danilo Krummrich
2024-07-04 18:36 ` [PATCH v2 02/37] drm/nouveau: handle pci/tegra drm_dev_{alloc, register} from common code Ben Skeggs
2024-07-09 15:16   ` Danilo Krummrich
2024-07-18  7:14     ` Ben Skeggs
2024-07-19 11:10       ` Danilo Krummrich
2024-07-26  4:27         ` Ben Skeggs
2024-07-26 15:41           ` Danilo Krummrich
2024-07-26 13:07             ` Ben Skeggs
2024-07-27  1:54               ` Danilo Krummrich
2024-07-28 18:13               ` Jason Gunthorpe
2024-07-28 21:34                 ` Danilo Krummrich
2024-07-28 23:04                   ` Jason Gunthorpe
2024-07-29  0:55                     ` Danilo Krummrich
2024-07-04 18:36 ` [PATCH v2 03/37] drm/nouveau: replace drm_device* with nouveau_drm* as dev drvdata Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 04/37] drm/nouveau: create pci device once Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 05/37] drm/nouveau: store nvkm_device pointer in nouveau_drm Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 06/37] drm/nouveau: move allocation of root client out of nouveau_cli_init() Ben Skeggs
2024-07-09 15:33   ` Danilo Krummrich
2024-07-18  7:29     ` Ben Skeggs
2024-07-19 11:37       ` Danilo Krummrich
2024-07-26  4:29         ` Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 07/37] drm/nouveau: add nouveau_cli to nouveau_abi16 Ben Skeggs
2024-07-09 15:36   ` Danilo Krummrich
2024-07-04 18:36 ` [PATCH v2 08/37] drm/nouveau: handle limited nvif ioctl in abi16 Ben Skeggs
2024-07-09 16:03   ` Danilo Krummrich
2024-07-18  7:43     ` Ben Skeggs
2024-07-19 12:06       ` Danilo Krummrich
2024-07-04 18:36 ` [PATCH v2 09/37] drm/nouveau: remove abi16->device Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 10/37] drm/nouveau: remove abi16->handles Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 11/37] drm/nouveau/nvkm: remove detect/mmio/subdev_mask from device args Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 12/37] drm/nouveau/nvkm: remove perfmon Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 13/37] drm/nouveau/nvkm: remove nvkm_client_search() Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 14/37] drm/nouveau/nvif: remove support for userspace backends Ben Skeggs
2024-07-04 18:36 ` [PATCH v2 15/37] drm/nouveau/nvif: remove route/token Ben Skeggs
2024-07-09 16:11   ` Danilo Krummrich
2024-07-18  7:52     ` Ben Skeggs
2024-07-19 12:12       ` Danilo Krummrich [this message]
2024-07-04 18:37 ` [PATCH v2 16/37] drm/nouveau/nvif: remove nvxx_object() Ben Skeggs
2024-07-09 16:14   ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 17/37] drm/nouveau/nvif: remove nvxx_client() Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 18/37] drm/nouveau/nvif: remove driver keep/fini Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 19/37] drm/nouveau/nvif: remove client device arg Ben Skeggs
2024-07-09 16:16   ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 20/37] drm/nouveau/nvif: remove client version Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 21/37] drm/nouveau/nvif: remove client devlist Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 22/37] drm/nouveau/nvif: remove client fini Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 23/37] drm/nouveau/nvif: remove device args Ben Skeggs
2024-07-09 16:18   ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 24/37] drm/nouveau: always map device Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 25/37] drm/nouveau/nvif: remove device rd/wr Ben Skeggs
2024-07-09 16:22   ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 26/37] drm/nouveau/nvif: remove disp chan rd/wr Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 27/37] drm/nouveau: move nvxx_* definitions to nouveau_drv.h Ben Skeggs
2024-07-09 16:31   ` Danilo Krummrich
2024-07-18  7:58     ` Ben Skeggs
2024-07-19 12:28       ` Danilo Krummrich
2024-07-26  4:35         ` Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 28/37] drm/nouveau: add nvif_mmu to nouveau_drm Ben Skeggs
2024-07-09 16:34   ` Danilo Krummrich
2024-07-18  8:10     ` Ben Skeggs
2024-07-19 12:47       ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 29/37] drm/nouveau: pass drm to nouveau_mem_new(), instead of cli Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 30/37] drm/nouveau: pass drm to nv50_dmac_create(), rather than device+disp Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 31/37] drm/nouveau: pass cli to nouveau_channel_new() instead of drm+device Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 32/37] drm/nouveau: remove nouveau_chan.device Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 33/37] drm/nouveau: remove chan->drm Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 34/37] drm/nouveau: remove master Ben Skeggs
2024-07-09 16:38   ` Danilo Krummrich
2024-07-18  8:12     ` Ben Skeggs
2024-07-19 12:49       ` Danilo Krummrich
2024-07-04 18:37 ` [PATCH v2 35/37] drm/nouveau: remove push pointer from nouveau_channel Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 36/37] drm/nouveau/kms: remove a few unused struct members and fn decls Ben Skeggs
2024-07-04 18:37 ` [PATCH v2 37/37] drm/nouveau/kms: remove push pointer from nv50_dmac Ben Skeggs
2024-07-09 14:44 ` [PATCH v2 00/37] drm/nouveau: misc. cleanups and removal of unused apis Danilo Krummrich
2024-07-18  7:00   ` Ben Skeggs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZppYOmWfup8lwC3L@pollux \
    --to=dakr@redhat.com \
    --cc=bskeggs@nvidia.com \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.