linux-kernel - Re: [PATCH 2/4] WIP: drm: Introduce rvkms

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]

Date: Sun, 21 Apr 2024 21:54:25 -0400
From: Lyude Paul <lyude@...hat.com>
To: Benno Lossin <benno.lossin@...ton.me>, dri-devel@...ts.freedesktop.org
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
 <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, Miguel Ojeda
 <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida
 Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...sung.com>,
 Alice Ryhl <aliceryhl@...gle.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/4] WIP: drm: Introduce rvkms

On Wed, 2024-03-27 at 21:06 +0000, Benno Lossin wrote:
> On 22.03.24 23:03, Lyude Paul wrote:
> > diff --git a/drivers/gpu/drm/rvkms/connector.rs
> > b/drivers/gpu/drm/rvkms/connector.rs
> > new file mode 100644
> > index 0000000000000..40f84d38437ee
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rvkms/connector.rs
> > @@ -0,0 +1,55 @@
> > +// TODO: License and stuff
> > +// Contain's rvkms's drm_connector implementation
> > +
> > +use super::{RvkmsDriver, RvkmsDevice, MAX_RES, DEFAULT_RES};
> > +use kernel::{
> > +    prelude::*,
> > +    drm::{
> > +        device::Device,
> > +        kms::{
> > +            connector::{self, ConnectorGuard},
> > +            ModeConfigGuard
> > +        }
> > +    },
> > +    prelude::*
> > +};
> > +use core::marker::PhantomPinned;
> > +
> > +#[pin_data]
> > +pub(crate) struct DriverConnector {
> > +    #[pin]
> > +    _p: PhantomPinned
> > +}
> 
> This struct does not need to be annotated with `#[pin_data]`, this
> should just work:
> 
> pub(crate) struct DriverConnector;
> 
> > +
> > +pub(crate) type Connector = connector::Connector<DriverConnector>;
> > +
> > +impl connector::DriverConnector for DriverConnector {
> > +    type Initializer = impl PinInit<Self, Error>;
> > +
> > +    type State = ConnectorState;
> > +
> > +    type Driver = RvkmsDriver;
> > +
> > +    type Args = ();
> > +
> > +    fn new(dev: &Device<Self::Driver>, args: Self::Args) ->
> > Self::Initializer {
> 
> And then here just return `Self`.
> 
> This works, since there is a blanket impl `PinInit<T, E> for T`.
> 
> Looking at how you use this API, I am not sure if you actually need
> pin-init for the type that implements `DriverConnector`.
> Do you need to store eg `Mutex<T>` or something else that needs
> pin-init in here in a more complex driver?

Most likely yes - a lot of drivers have various private locks contained
within their subclassed mode objects. I'm not sure we will in rvkms's
connector since vkms doesn't really do much with connectors - but we at
a minimum be using pinned types (spinlocks and hrtimers) in our
DriverCrtc implementation once I've started implementing support for
vblanks[1]

[1]
https://www.kernel.org/doc/html/v6.9-rc5/gpu/drm-kms.html?highlight=vblank#vertical-blanking

In nova (the main reason I'm working on rvkms in the first place),
we'll definitely have locks in our connectors and possibly other types.

> 

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