From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D6884655F; Mon, 15 Jul 2024 17:06:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=46.235.227.194 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721063169; cv=none; b=XJU/svh+4/r8FxXGBrK/dWqX1brbkK7zoKAYg/YvPjniWyGMsTvizAE0fTJtDqEdeMsR+di4zzr4DWcfl0w8Y7p8OL2FQb3jg5cWDBvEJ7K6XcXBzGI/fsRuOyPap+UR7uktLU9pBANcB9yCjKe0SD2QJYyVmS5DKbmO5dCAqJg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721063169; c=relaxed/simple; bh=MTzvTzAoatx0Q48KY1FTPa7YgzLwWXSImXnZ/96XLmY=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=oQwgxQw+aNylYascDKgN1tqu8hPHyNTbAtTsR7ERc2MLgoTC5n/vsa8OzlADSexZQNsAvUaQeUWViQ5hWPTubvFnusyhLMdFv6IKHU6HbDPbHVzYwWfD74S7vCbF/K+3AOdngBWkbY8a2nbG439svfnEUQevk+QL7qOpTYb4YqQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=IPabq0Aj; arc=none smtp.client-ip=46.235.227.194 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="IPabq0Aj" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1721063166; bh=MTzvTzAoatx0Q48KY1FTPa7YgzLwWXSImXnZ/96XLmY=; h=Subject:From:In-Reply-To:Date:Cc:References:To:From; b=IPabq0AjwORpneio+GIUvHaQSCAGrJxwa4XH0288QESrg/JDPfdvTcw0TIUoWZ+zh V5N3dU3vYbTpiOWgFuvvW+xi+p5Vttwj3LZAonH6q34e0XPPf8JyeBMxc+UJZOLzVw Kkbto8N2n9zDuxcu4Ds59NUVCVGXADRu58SbzVTkC1A+fxjyDI5IN2BZwaQHbYU0KW vZkOwKM7pVie7w36ao5c/3TipklCyhDNrI2XOUi5WUBvWQpsxcvZKoKca3r7c/VQMx xgqyd9e0ZaTO95kUmMtXsz9uLcEhkAh44XdJIT/BgNSBMn2F45vwYrrHCBNNNb7+bn juoK9dOvmCyQA== Received: from smtpclient.apple (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: dwlsalmeida) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 7E4A23782193; Mon, 15 Jul 2024 17:06:02 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.600.62\)) Subject: Re: [RFC PATCH] drm: panthor: add dev_coredumpv support From: Daniel Almeida In-Reply-To: Date: Mon, 15 Jul 2024 14:05:49 -0300 Cc: Danilo Krummrich , Wedson Almeida Filho , ojeda@kernel.org, lyude@redhat.com, robh@kernel.org, lina@asahilina.net, mcanal@igalia.com, airlied@gmail.com, "daniel@ffwll.ch" , rust-for-linux@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <89FFB898-120A-431E-9B18-4D08AFAEFBFD@collabora.com> References: <20240710225011.275153-1-daniel.almeida@collabora.com> To: Daniel Vetter X-Mailer: Apple Mail (2.3774.600.62) Hi Sima! >=20 > Yeah I'm not sure a partially converted driver where the main driver = is > still C really works, that pretty much has to throw out all the type > safety in the interfaces. >=20 > What I think might work is if such partial drivers register as full = rust > drivers, and then largely delegate the implementation to their = existing C > code with a big "safety: trust me, the C side is bug free" comment = since > it's all going to be unsafe :-) >=20 > It would still be a big change, since all the driver's callbacks need = to > switch from container_of to upcast to their driver structure to some = small > rust shim (most likely, I didn't try this out) to get at the driver = parts > on the C side. And I think you also need a small function to downcast = to > the drm base class. But that should be all largely mechanical. >=20 > More freely allowing to mix&match is imo going to be endless pains. We > kinda tried that with the atomic conversion helpers for legacy kms > drivers, and the impendance mismatch was just endless amounts of very > subtle pain. Rust will exacerbate this, because it encodes semantics = into > the types and interfaces. And that was with just one set of helpers, = for > rust we'll likely need a custom one for each driver that's partially > written in rust. > -Sima >=20 I humbly disagree here. I know this is a bit tangential, but earlier this year I converted a = bunch of codec libraries to Rust in v4l2. That worked just fine with the = C codec drivers. There were no regressions as per our test tools. The main idea is that you isolate all unsafety to a single point: so = long as the C code upholds the safety guarantees when calling into Rust, = the Rust layer will be safe. This is just the same logic used in unsafe = blocks in Rust itself, nothing new really. This is not unlike what is going on here, for example: ``` +unsafe extern "C" fn open_callback, U: = BaseObject>( + raw_obj: *mut bindings::drm_gem_object, + raw_file: *mut bindings::drm_file, +) -> core::ffi::c_int { + // SAFETY: The pointer we got has to be valid. + let file =3D unsafe { + file::File::<<::Driver as = drv::Driver>::File>::from_raw(raw_file) + }; + let obj =3D + <<::Driver as drv::Driver>::Object as = IntoGEMObject>::from_gem_obj( + raw_obj, + ); + + // SAFETY: from_gem_obj() returns a valid pointer as long as the type = is + // correct and the raw_obj we got is valid. + match T::open(unsafe { &*obj }, &file) { + Err(e) =3D> e.to_errno(), + Ok(()) =3D> 0, + } +} ``` We have to trust that the kernel is passing in a valid pointer. By the = same token, we can choose to trust drivers if we so desire. > that pretty much has to throw out all the type > safety in the interfaces. Can you expand on that? In particular, I believe that we should ideally be able to convert from = a C "struct Foo * " to a Rust =E2=80=9CFooRef" for types whose lifetimes = are managed either by the kernel itself or by a C driver. In practical = terms, this has run into the issues we=E2=80=99ve been discussing in = this thread, but there may be solutions e.g.: > One thing that comes to my mindis , you could probably create some = driver specific > "dummy" types to satisfy the type generics of the types you want to = use. Not sure > how well this works out though. I haven=E2=80=99t thought of anything yet - which is why I haven=E2=80=99t= replied. OTOH, IIRC, Faith seems to have something in mind that can = work with the current abstractions, so I am waiting on her reply. > What I think might work is if such partial drivers register as full = rust > drivers, and then largely delegate the implementation to their = existing C > code with a big "safety: trust me, the C side is bug free" comment = since > it's all going to be unsafe :-) > with a big "safety: trust me, the C side is bug free" comment since = it's all going to be unsafe :-) This is what I want too :) but I can=E2=80=99t see how your proposed = approach is better, at least at a cursory glance. It is a much bigger = change, though, which is a clear drawback. > And that was with just one set of helpers, for > rust we'll likely need a custom one for each driver that's partially > written in rust. That=E2=80=99s exactly what I am trying to avoid. In other words, I want = to find a way to use the same abstractions and the same APIs so that we = do not run precisely into that problem. =E2=80=94 Daniel