linux-kernel - Re: [PATCH drm-misc-next v4 6/8] drm/gpuvm: add drm_gpuvm_flags to drm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230922135842.242c865d@collabora.com>
Date:   Fri, 22 Sep 2023 13:58:42 +0200
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     Danilo Krummrich <dakr@...hat.com>
Cc:     airlied@...il.com, daniel@...ll.ch, matthew.brost@...el.com,
        thomas.hellstrom@...ux.intel.com, sarah.walker@...tec.com,
        donald.robson@...tec.com, christian.koenig@....com,
        faith.ekstrand@...labora.com, dri-devel@...ts.freedesktop.org,
        nouveau@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH drm-misc-next v4 6/8] drm/gpuvm: add drm_gpuvm_flags to
 drm_gpuvm

On Wed, 20 Sep 2023 16:42:39 +0200
Danilo Krummrich <dakr@...hat.com> wrote:

> +/**
> + * enum drm_gpuvm_flags - flags for struct drm_gpuvm
> + */
> +enum drm_gpuvm_flags {
> +	/**
> +	 * @DRM_GPUVM_USERBITS: user defined bits
> +	 */
> +	DRM_GPUVM_USERBITS = (1 << 0),

Nit: I tried declaring driver-specific flags, and I find this
counter-intuitive. You basically end up with something like:

enum my_gpuvm_flags {
	MY_FLAG_X = DRM_GPUVM_USERBITS,
	MY_FLAG_Y = DRM_GPUVM_USERBITS << 1,
};

instead of the usual

enum my_gpuvm_flags {
	MY_FLAG_X = BIT(0),
	MY_FLAG_Y = BIT(1),
};

pattern.

Another issue I see coming is if we end up adding more core flags and
drivers start falling short of bits for their own flags. This makes me
wonder if we shouldn't kill this notion of USER flags and let drivers
store their flags in some dedicated field, given they're likely to
derive drm_gpuvm and drm_gpuva with their own object anyway.

> +};
> +