All of lore.kernel.org
 help / color / mirror / Atom feed
From: Deborah Brouwer <deborah.brouwer@collabora.com>
To: Lyude Paul <lyude@redhat.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org,
	Danilo Krummrich <dakr@kernel.org>,
	nouveau@lists.freedesktop.org,
	Daniel Almeida <daniel.almeida@collabora.com>,
	Gary Guo <gary@garyguo.net>, Benno Lossin <lossin@kernel.org>,
	Alexandre Courbot <acourbot@nvidia.com>,
	Janne Grunau <j@jannau.net>
Subject: Re: [PATCH v7 6/7] rust: Introduce iosys_map bindings
Date: Thu, 12 Feb 2026 17:59:09 -0800	[thread overview]
Message-ID: <aY6FbSjQpGFk8oWK@um790> (raw)
In-Reply-To: <20260206223431.693765-7-lyude@redhat.com>

Hi Lyude,

On Fri, Feb 06, 2026 at 05:34:30PM -0500, Lyude Paul wrote:
> This introduces a set of bindings for working with iosys_map in rust code
> using the new Io traits.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> ---
> V5:
> - Fix incorrect field size being passed to iosys_map_memcpy_to()
> - Add an additional unit test, basic_macro(), which can successfully catch
>   the above issue so it doesn't happen again in the future.
> V6:
> - Drop as_slice/as_mut_slice (Alice Rhyl)
> V7:
> - Start using Alexandre Courbot's wonderful Io, IoCapable and IoKnownSize
>   traits instead of trying to roll our own IO accessors. This also changes
>   the following:
>   - We don't have a custom AsBytes/FromBytes type that we carry around any
>     longer with maps
>   - We now have optional compile-time size checking
>   - We don't need our own unit tests anymore
>   - RawIoSysMap can be used for unsafe IO operations, because why not.
>   - IoSysMapRef::new() can fail now since it needs to ensure SIZE is valid.
>   - We don't implement Deref<RawIoSysMap> for IoSysMapRef any longer. The
>     main reason for this is that we want to avoid users having to manually
>     specify if they want the RawIoSysMap or IoSysMapRef variant functions
>     like io_read()/io_write().
>     This is fine IMHO, but to make sure things remain easy for APIs that
>     wrap around iosys map we make IoSysMapRef.raw_map pub(crate).
> 
>  rust/helpers/helpers.c   |   1 +
>  rust/helpers/iosys_map.c |  34 +++++++
>  rust/kernel/iosys_map.rs | 211 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs       |   1 +
>  4 files changed, 247 insertions(+)
>  create mode 100644 rust/helpers/iosys_map.c
>  create mode 100644 rust/kernel/iosys_map.rs
> 
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 1d3333cc0d2a8..bd8ad237aa02e 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -31,6 +31,7 @@
>  #include "irq.c"
>  #include "fs.c"
>  #include "io.c"
> +#include "iosys_map.c"
>  #include "jump_label.c"
>  #include "kunit.c"
>  #include "maple_tree.c"
> diff --git a/rust/helpers/iosys_map.c b/rust/helpers/iosys_map.c
> new file mode 100644
> index 0000000000000..6861d4ec48a4b
> --- /dev/null
> +++ b/rust/helpers/iosys_map.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/iosys-map.h>
> +#include <linux/types.h>
> +
> +#define rust_iosys_map_rd(type__)                                                       \
> +	__rust_helper type__                                                            \
> +	rust_helper_iosys_map_rd_ ## type__(const struct iosys_map *map, size_t offset) \
> +	{                                                                               \
> +		return iosys_map_rd(map, offset, type__);                               \
> +	}
> +#define rust_iosys_map_wr(type__)                                                       \
> +	__rust_helper void                                                              \
> +	rust_helper_iosys_map_wr_ ## type__(const struct iosys_map *map, size_t offset, \
> +					    type__ value)                               \
> +	{                                                                               \
> +		iosys_map_wr(map, offset, type__, value);                               \
> +	}
> +
> +rust_iosys_map_rd(u8);
> +rust_iosys_map_rd(u16);
> +rust_iosys_map_rd(u32);
> +
> +rust_iosys_map_wr(u8);
> +rust_iosys_map_wr(u16);
> +rust_iosys_map_wr(u32);
> +
> +#ifdef CONFIG_64BIT
> +rust_iosys_map_rd(u64);
> +rust_iosys_map_wr(u64);
> +#endif
> +
> +#undef rust_iosys_map_rd
> +#undef rust_iosys_map_wr
> diff --git a/rust/kernel/iosys_map.rs b/rust/kernel/iosys_map.rs
> new file mode 100644
> index 0000000000000..2070f0fb42cb8
> --- /dev/null
> +++ b/rust/kernel/iosys_map.rs
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! IO-agnostic memory mapping interfaces.
> +//!
> +//! This crate provides bindings for the `struct iosys_map` type, which provides a common interface
> +//! for memory mappings which can reside within coherent memory, or within IO memory.
> +//!
> +//! C header: [`include/linux/iosys-map.h`](srctree/include/linux/pci.h)
> +
> +use crate::{
> +    error::code::*,
> +    io::{
> +        Io,
> +        IoCapable,
> +        IoKnownSize, //
> +    },
> +    prelude::*, //
> +};
> +use bindings;
> +use core::marker::PhantomData;
> +
> +/// Raw unsized representation of a `struct iosys_map`.
> +///
> +/// This struct is a transparent wrapper around `struct iosys_map`. The C API does not provide the
> +/// size of the mapping by default, and thus this type also does not include the size of the
> +/// mapping. As such, it cannot be used for actually accessing the underlying data pointed to by the
> +/// mapping.
> +///
> +/// With the exception of kernel crates which may provide their own wrappers around `RawIoSysMap`,
> +/// users will typically not interact with this type directly.
> +#[repr(transparent)]
> +pub struct RawIoSysMap<const SIZE: usize = 0>(bindings::iosys_map);
> +
> +impl<const SIZE: usize> RawIoSysMap<SIZE> {
> +    /// Convert from a raw `bindings::iosys_map`.
> +    #[expect(unused)]
> +    #[inline]
> +    pub(crate) fn from_raw(val: bindings::iosys_map) -> Self {
> +        Self(val)
> +    }
> +
> +    /// Returns whether the mapping is within IO memory space or not.
> +    #[inline]
> +    pub fn is_iomem(&self) -> bool {
> +        self.0.is_iomem
> +    }
> +
> +    /// Convert from a `RawIoSysMap<SIZE>` to a raw `bindings::iosys_map` ref.
> +    #[expect(unused)]
> +    #[inline]
> +    pub(crate) fn as_raw(&self) -> &bindings::iosys_map {
> +        &self.0
> +    }
> +
> +    /// Convert from a `RawIoSysMap<SIZE>` to a raw mutable `bindings::iosys_map` ref.
> +    #[allow(unused)]
> +    #[inline]
> +    pub(crate) fn as_raw_mut(&mut self) -> &mut bindings::iosys_map {
> +        &mut self.0
> +    }
> +
> +    /// Returns the address pointed to by this iosys map.
> +    ///
> +    /// Note that this address is not guaranteed to be valid, and may or may not reside in I/O
> +    /// memory.
> +    #[inline]
> +    pub fn addr(&self) -> usize {
> +        (if self.is_iomem() {
> +            // SAFETY: We confirmed above that this iosys map is contained within iomem, so it's
> +            // safe to read vaddr_iomem
> +            unsafe { self.0.__bindgen_anon_1.vaddr_iomem }
> +        } else {
> +            // SAFETY: We confirmed above that this iosys map is not contaned within iomem, so it's
> +            // safe to read vaddr.
> +            unsafe { self.0.__bindgen_anon_1.vaddr }
> +        }) as usize
> +    }
> +}
> +
> +// SAFETY: As we make no guarantees about the validity of the mapping, there's no issue with sending
> +// this type between threads.
> +unsafe impl<const SIZE: usize> Send for RawIoSysMap<SIZE> {}
> +
> +impl<const SIZE: usize> Clone for RawIoSysMap<SIZE> {
> +    fn clone(&self) -> Self {
> +        Self(self.0)
> +    }
> +}
> +
> +macro_rules! impl_raw_iosys_map_io_capable {
> +    ($ty:ty, $read_fn:ident, $write_fn:ident) => {
> +        impl<const SIZE: usize> IoCapable<$ty> for RawIoSysMap<SIZE> {
> +            #[inline(always)]
> +            unsafe fn io_read(&self, address: usize) -> $ty {
> +                // SAFETY: By the trait invariant `address` is a valid address for iosys map
> +                // operations.
> +                unsafe { bindings::$read_fn(&self.0, address) }
> +            }
> +
> +            #[inline(always)]
> +            unsafe fn io_write(&self, value: $ty, address: usize) {
> +                // SAFETY: By the trait invariant `address` is a valid address for iosys map
> +                // operations.
> +                unsafe { bindings::$write_fn(&self.0, address, value) };
> +            }
> +        }
> +    };
> +}
> +

I think there might be a mismatch between the absolute address being
passed to these read and write functions and the bindings helpers
that expect an offset argument.

This crashed Tyr when I tried to write the firmware in u8 chunks at
incremental offsets. But if i just calculated the offset and passed that
instead of the absolute address, this worked fine:

  let offset = address - self.addr();
  unsafe { bindings::$write_fn(&self.0, offset, value) };

Here's some of the call trace:

[   31.553727] tyr fb000000.gpu: supply sram not found, using dummy regulator
[   31.555096] tyr fb000000.gpu: mali-unknown id 0xa867 major 0x67 minor 0x0 status 0x5
[   31.555778] tyr fb000000.gpu: Features: L2:0x7120306 Tiler:0x809 Mem:0x301 MMU:0x2830 AS:0xff
[   31.556521] tyr fb000000.gpu: shader_present=0x0000000000050005 l2_present=0x0000000000000001 tiler_present=0x0000000000000001
[   31.557868] stackdepot: allocating hash table of 524288 entries via kvcalloc
[  OK  ] Started systemd-tmpfiles-clean.tim…y Cleanup of Temporary Directories.
[   31.562424] stackdepot: allocating space for 8192 stack pools via kvcalloc
[  OK  ] Reached target timers.target - Timer Units.
[   31.563676] tyr: Firmware protected mode entry not supported, ignoring
[   31.571391] Unable to handle kernel paging request at virtual address 0000000000800069
[   31.572762] Mem abort info:
[   31.573027]   ESR = 0x0000000096000021
[   31.573402]   EC = 0x25: DABT (current EL), IL = 32 bits
[   31.573878]   SET = 0, FnV = 0
[   31.574157]   EA = 0, S1PTW = 0
[   31.574442]   FSC = 0x21: alignment fault
[  OK  ] Listening on dbus.socket - D-Bus System Message Bus Socket.
[   31.593348] Data abort info:
[   31.593628]   ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
[   31.594117]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   31.594567]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   31.595042] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000105e50000
[   31.595613] [0000000000800069] pgd=0000000000000000, p4d=0000000000000000
[   31.596434] Internal error: Oops: 0000000096000021 [#1]  SMP
[   31.596936] Modules linked in: snd tyr(+) soundcore sha256 cfg80211 rfkill pci_endpoint_test fuse dm_mod ipv6
[   31.597830] CPU: 3 UID: 0 PID: 241 Comm: (udev-worker) Not tainted 6.19.0-rc5 #282 PREEMPT
[   31.598561] Hardware name: Radxa ROCK 5B (DT)
[   31.598944] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   31.599554] pc : __d_lookup_rcu+0xbc/0x168
[   31.599921] lr : __d_lookup_rcu+0x60/0x168
[   31.600283] sp : ffff800081ebba00
[   31.600574] x29: ffff800081ebba00 x28: 0000000000000049 x27: ffff00010d07002b
[   31.601205] x26: 0000000000000081 x25: 000000000006915e x24: ffff0002f6600000
[   31.601848] x23: ffff00010396a040 x22: ffff000102098460 x21: 000000036915e207
[   31.601859] x20: ffff000101e10af8 x19: ffff800081ebbcac x18: 2f64662f666c6573
[   31.601867] x17: ffffff00666c6573 x16: 00000000fffffffc x15: 0000000000003431
[   31.601875] x14: 000000000000000f x13: 0080205100800107 x12: 0000000000800069
[   31.601882] x11: ffffffffffffffff x10: 0000000000000018 x9 : 0000000000000003
[   31.601890] x8 : 00000000008000a1 x7 : ffffb9d13df173c8 x6 : 0000000000000000
[   31.601897] x5 : 0000000000000000 x4 : 0000000000000010 x3 : ffffffffffff0a00
[   31.601905] x2 : ffff800081ebbcac x1 : ffffb9d1406be718 x0 : 0000000000000001
[   31.601913] Call trace:
[   31.601915]  __d_lookup_rcu+0xbc/0x168 (P)
[   31.601922]  lookup_fast+0x98/0x174
[   31.601929]  link_path_walk+0x280/0x528
[   31.601935]  path_lookupat+0x60/0x1f0
[   31.601941]  do_o_path+0x34/0xb4
[   31.601947]  path_openat+0xccc/0xe34
[   31.601953]  do_filp_open+0xc0/0x170
[   31.601958]  do_sys_openat2+0x88/0x10c
[   31.601963]  __arm64_sys_openat+0x70/0x9c
[   31.601968]  invoke_syscall+0x40/0xcc
[   31.601974]  el0_svc_common+0x80/0xd8
[   31.601979]  do_el0_svc+0x1c/0x28
[   31.601984]  el0_svc+0x54/0x1d4
[   31.601991]  el0t_64_sync_handler+0x84/0x12c
[   31.601997]  el0t_64_sync+0x198/0x19c
[   31.602005] Code: 14000003 f9400108 b4000428 d100e10c (88dffd8c)
[   31.602009] ---[ end trace 0000000000000000 ]---
[  OK  ] Listening on sshd-unix-local.socke…temd-ssh-generator, AF_UNIX Local).
[   31.620067] mc: Linux media interface: v0.10
[   31.623483] [drm] Initialized panthor 1.5.0 for fb000000.gpu on minor 0
[   31.623765] Unable to handle kernel paging request at virtual address 00802a4d0080284d
[   31.624285] tyr fb000000.gpu: Tyr initialized correctly.
[   31.624752] Mem abort info:
[   31.624754]   ESR = 0x0000000096000004
[   31.625847]   EC = 0x25: DABT (current EL), IL = 32 bits
[   31.626310]   SET = 0, FnV = 0
[   31.626578]   EA = 0, S1PTW = 0
[   31.626853]   FSC = 0x04: level 0 translation fault
[   31.627277] Data abort info:
[   31.627529]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   31.628006]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   31.628447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   31.628910] [00802a4d0080284d] address between user and kernel address ranges
[   31.629535] Internal error: Oops: 0000000096000004 [#2]  SMP
[   31.630030] Modules linked in: mc drm_client_lib snd tyr soundcore sha256 cfg80211 rfkill pci_endpoint_test fuse dm_mod ipv6
[   31.631017] CPU: 4 UID: 0 PID: 225 Comm: systemd-udevd Tainted: G      D             6.19.0-rc5 #282 PREEMPT
[   31.631877] Tainted: [D]=DIE
[   31.632129] Hardware name: Radxa ROCK 5B (DT)
[   31.632506] pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   31.633111] pc : ___d_drop+0xd8/0x144
[   31.633433] lr : d_invalidate+0x3c/0x110
[   31.633776] sp : ffff800081d3ba70
[   31.634064] x29: ffff800081d3ba90 x28: 0000000000000001 x27: ffffb9d140bfa000
[   31.634685] x26: ffff0001039b5108 x25: ffff00010396a000 x24: ffffb9d140166275
[   31.635305] x23: ffffb9d140bfa000 x22: ffffb9d140152f3a x21: ffff000104076000
[   31.635925] x20: ffff00010e520098 x19: ffff00010396a000 x18: 0000000000000000
[   31.636545] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[   31.637165] x14: 0000000000000000 x13: ffff00010792b8f0 x12: 000000000000017a
[   31.637785] x11: 00000000008000a1 x10: 00802a4d0080284d x9 : ffff0002f6604010
[   31.638405] x8 : ffff000105bfcd40 x7 : 0000000000000000 x6 : ffffb9d13e272c60
[   31.639026] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000000
[   31.639646] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00010396a000
[   31.640266] Call trace:
[   31.640479]  ___d_drop+0xd8/0x144 (P)
[   31.640800]  d_invalidate+0x3c/0x110
[   31.641113]  proc_invalidate_siblings_dcache+0x244/0x2b8
[   31.641577]  proc_flush_pid+0x1c/0x28
[   31.641897]  release_task+0x560/0x668
[   31.642218]  wait_consider_task+0x5a0/0xb44
[   31.642582]  __do_wait+0x154/0x1f0
[   31.642879]  do_wait+0x84/0x16c
[   31.643154]  __arm64_sys_waitid+0xac/0x218
[   31.643512]  invoke_syscall+0x40/0xcc
[   31.643833]  el0_svc_common+0x80/0xd8
[   31.644152]  do_el0_svc+0x1c/0x28
[   31.644441]  el0_svc+0x54/0x1d4
[   31.644718]  el0t_64_sync_handler+0x84/0x12c
[   31.645090]  el0t_64_sync+0x198/0x19c
[   31.645412] Code: 5280002a f85f83a0 17fffff1 a944280b (f940014c)
[   31.645941] ---[ end trace 0000000000000000 ]---

thanks,
Deborah


  parent reply	other threads:[~2026-02-13  1:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06 22:34 [PATCH v7 0/7] Rust bindings for gem shmem + iosys_map Lyude Paul
2026-02-06 22:34 ` [PATCH v7 1/7] rust: drm: gem: Add raw_dma_resv() function Lyude Paul
2026-02-09 12:40   ` Alice Ryhl
2026-02-09 12:47     ` Janne Grunau
2026-02-06 22:34 ` [PATCH v7 2/7] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
2026-02-06 22:34 ` [PATCH v7 3/7] rust: gem: Introduce DriverObject::Args Lyude Paul
2026-02-06 22:34 ` [PATCH v7 4/7] rust: drm: gem: shmem: Add DRM shmem helper abstraction Lyude Paul
2026-02-17 19:38   ` Daniel Almeida
2026-02-06 22:34 ` [PATCH v7 5/7] rust: drm: gem: Introduce shmem::SGTable Lyude Paul
2026-02-17 19:53   ` Daniel Almeida
2026-02-06 22:34 ` [PATCH v7 6/7] rust: Introduce iosys_map bindings Lyude Paul
2026-02-09 11:17   ` Danilo Krummrich
2026-02-09 11:17     ` Danilo Krummrich
2026-02-13  1:59   ` Deborah Brouwer [this message]
2026-03-04 22:59     ` lyude
2026-03-06 20:59       ` Deborah Brouwer
2026-02-06 22:34 ` [PATCH v7 7/7] rust: drm/gem: Add vmap functions to shmem bindings Lyude Paul

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=aY6FbSjQpGFk8oWK@um790 \
    --to=deborah.brouwer@collabora.com \
    --cc=acourbot@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=j@jannau.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=rust-for-linux@vger.kernel.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.