Skip to content

PCI_IOC_MAP_BAR ioctl for userspace BAR mapping - #74

Draft
OliverMiyarUgarte wants to merge 10000 commits into
NetBSD:trunkfrom
OliverMiyarUgarte:userland-pci
Draft

PCI_IOC_MAP_BAR ioctl for userspace BAR mapping#74
OliverMiyarUgarte wants to merge 10000 commits into
NetBSD:trunkfrom
OliverMiyarUgarte:userland-pci

Conversation

@OliverMiyarUgarte

Copy link
Copy Markdown

PCI_IOC_MAP_BAR ioctl for userspace BAR mapping

Added a new ioctl to /dev/pci that allows userspace drivers to map PCI BARs.

What was done:

  • Adds struct pciio_bar_map to pciio.h
  • Implements PCI_IOC_MAP_BAR handler in pci_usrreq.c

Testing performed:

  • Tested with QEMU's edu device
  • Mapped BAR0 and read device ID register
  • Verified correct size and offset from kernel ranges

What's Next:

  • Expose PCI interrupts as events that can be read from a pci(4) (/dev/pciN) device instance, and use that to implement the pci_intr(9) API in userland.
  • Devise a scheme for userland allocate and map memory for DMA in order to implement bus_dma(9).

Note: This is a draft PR to get early feedback before adding documentation and final polish. All feedback welcome!

kre and others added 30 commits February 2, 2026 21:16
Note, this is not really a standards issue, POSIX says:

	If the -m option is specified, the number of characters shall
	replace the <bytes> field in this format.

There is no output format in POSIX which permits both bytes and
characters to be specified, and since which value represents which
counter is determined entirely by position, which of bytes or
characters would come first if both were to be presented would be
indeterminate, so I won't be doing that.

Slightly different wording used than that suggested bty Sebastian Ritter
in the PR.
Most workarounds for disabling static_assert declarations in lint mode
are no longer needed.
This driver uses bus_dmamap_sync correctly, so BUS_DMA_COHERENT should
not be needed.
If MAKE_VERSION is defined, create ${.MAKE.VERSION} read-only
and retain ${MAKE_VERSION} for backwards compatibility.
This might be unnecessary, but certainly doesn't hurt.
If the "VMware reserved" filesystem type was really intended to
have "vmresered" rather than "vmreserved" as its alias (for
gpt add -t vmreserved (etc)) then feel free to revert this change.
Turn on -r automatically if no write permission exists on device, and
it wasn't set on the command line, or implied by the command.
(allows other errors to be discovered before use of extra permissions,
and allows "gpt type -l <device>" (etc) with no write permission on device).

A better method of doing dry run experiments, leading to something
that might actually be useful (this much, by itself, isn't really)
is perhaps coming sometime later.
When we can (ie: when it is easy), avoid issuing 2 messages about
one problem.  One is enough.
Also, if the named file doesn't exist, create it,
again if -m is given (-m is almost never needed with
devices, the media size for them is generally obtained
from the device itself, so the risk of accidentally
creating /dev/rds2 is small...)

This allows

	gpt -m whatever-size create /tmp/TEST

to simply work, unless /tmp/TEST already exists and contains data.

After that, any other gpt commands can be applied (without -m being
needed).
In gpt_size_get() ensure that the size returned is a multiple
of the sector size.   This is automatic for unscaled numbers
(treated as multiples of the sector size) but in other cases
it is not automatic.   Note that even nK is not guartamteed to
be a multiple of the sector size if that happens to be 4K and
n is not a multiple of 4.
Note that we cannot use gpt_size_get() as that needs gpt->secsz
to have been set already (ie: can only be used after gpt_open)
and treats unscaled numbers as multiples of sectors, whereas the
-m arg has always been a byte size.

For gpt_human_get() or simply using dehumanize_number() we
would need to handle 's' and 'b' suffixes by hand anyway, so it
is simpler to simply to the whole thing longhand in this case.

This also allows (with some hackery) for the media size to be
set as some number of sectors, when we do not yet know the
sector size - it gets corrected later once the sector size has
been determined.
riastradh and others added 24 commits February 22, 2026 12:14
This call was originally introduced in sys/arch/amd64/amd64/mem.c
rev. 1.11 back in 2003 by fvdl@ with the commit message:

> Proper checks for kmem reads beyond _end

https://mail-index.netbsd.org/source-changes/2003/10/14/msg135803.html

At some point, SOMEBODY added an assertion in uvm that made it stop
working:

> uvm_map(9): Sprinkle assertions and interface contract comments.
>
> No functional change intended.

https://mail-index.netbsd.org/source-changes/2024/08/13/msg152763.html

(Really, this assertion revealed that this path was broken all along.)

This patch makes the amd64 mm_md_kernacc match the corresponding logic
in uvm_kernacc around uvm_map_checkprot:

    104 	vm_map_lock_read(kernel_map);
    105 	rv = uvm_map_checkprot(kernel_map, saddr, eaddr, prot);
    106 	vm_map_unlock_read(kernel_map);

https://nxr.netbsd.org/xref/src/sys/uvm/uvm_glue.c#92

Now, all of these checks in /dev/mem and company look like TOCTOU
bogosity, because the answers are stale by the time we reach uiomove,
which has to re-check any access anyway:

    269 	if (!md_kva) {
    270 		bool checked = false;
    271
    272 #ifdef __HAVE_MM_MD_KERNACC
    273 		/* MD check for the address. */
    274 		error = mm_md_kernacc(addr, prot, &checked);
    275 		if (error) {
    276 			return error;
    277 		}
    278 #endif
    279 		/* UVM check for the address (unless MD indicated to not). */
    280 		if (!checked && !uvm_kernacc(addr, len, prot)) {
    281 			return EFAULT;
    282 		}
    283 	}
    284 	error = uiomove(addr, len, uio);

https://nxr.netbsd.org/xref/src/sys/dev/mm.c?r=1.25#269

But we can address that in a separate change.

PR kern/59955: kernel diagnostic assertion "rw_lock_held(&map->lock)"
failed: file "/home/riastradh/netbsd/11/src/sys/uvm/uvm_map.c", line
1704
error: adding 'size_t' (aka 'unsigned long') to a string does not append
    to the string [-Werror,-Wstring-plus-int]
note: use array indexing to silence this warning
- don't use the glyph cache - it doesn't buy us anything when drawing mono
  characters and it can't deal with WSATTR_HILIT
- gffb_sync() before drawing characters - turns out we can overrun the command
  buffer with this, which leads to occasional lockups. No measurable loss of
  speed.
i had a system soft-hang with cc1plus waiting for memory, but there
is enough memory free now, but when i tried to "bt/a <cc1plus addr>"
i was getting an extremely large output (several megabytes before i
killed simh).

we fetch the argument count from a 32-bit space in the stack frame,
but it's only the bottom byte that matters, the other bytes should
always be zero for netbsd, as they're used by VMS.

mask the high bytes of this value before consuming it.

also, since we don't expect to see them, simply give up when we find
a "callg" instruction.  i did attempt to handle this, but since they
should not be present, and seeing one would indicate that an unexpected
stack write to that bit, handling it just leads to more faults as they
vax_ap pointer does not point to valid memory and triggers crash (ie,
back to db> prompt.)
rt_ifmsg will lock SPPP itself, so ensure it's unlocked before calling.
Add a comment to explain why this should be safe.

Thanks to @riastradh for review and commentary.

Fixes PR kern/59963.
Currently restricted to what I can test, so no OEA64 / OEA64_BRIDGE.
oea_dumpsys() recently grew support for writing crash dumps, so let's
call this directly. Hopefully fixes PR 11854.
As originally coded, the test intended to not set certain checksum
offload capabilities for the oldest RTL8152 chip revision couldn't
actually take effect. (The Realtek 4C00 device is an 8152 chipset,
thus URE_FLAG_8152 is also set, and the bitwise complement of
URE_FLAG_VER_4C00 isn't sufficient to test what's intended there.)

While here, also remove an inconsistently-applied INET6 check. Half the
relevant capabilities weren't guarded, and no other driver in our tree
guards IPv6 hardware checksumming this way, so follow suit here and
avoid the potential confusion/inconsistency.

Addresses PR kern/59997. Thanks to kre@ for significant input here.
Fix compilation of db_interface.c when KGDB is defined, but DDB is not.
Contains the fix for PR pkg/59991
For PR pkg/59991
…ctl nodes

with custom handlers in sysctl_dispatch()
I would also add a test case of sending and receiving a datagram, but
nc(1) doesn't seem to have a way to send or receive _one_ datagram;
as far as I can tell, it just loops forever on datagram sockets, with
no stopping point because there's no concept of FIN/shutdown().

PR kern/51963: sockets in chroot sandbox via null-mounts don't work
Add struct pciio_bar_map to pciio.h
@OliverMiyarUgarte

Copy link
Copy Markdown
Author

yamt pushed a commit to yamt/netbsd-src that referenced this pull request Mar 28, 2026
):

	sys/arch/evbppc/wii/dev/wiifb.c: revision 1.8
	sys/arch/evbppc/wii/dev/gxreg.h: revision 1.1
	sys/arch/evbppc/include/wii.h: revision 1.11
	sys/arch/evbppc/wii/machdep.c: revision 1.11
	sys/arch/evbppc/wii/dev/viio.h: file removal

wii: Use GX to accelerate RGB to YUY2 conversion.

Allocate a 640x480x32 RGB buffer in memory, and when not in EMUL mode,
use the CPU to copy from the RGB buffer to EFB (taking advantage of the
pixel engine to do RGB -> YUY2 for us) and then use the GX to copy from
EFB to XFB.

Not only does this improve performance significantly in X, it also results
in better colour reproduction. The previous software method employed by
xf86-video-wsfb had some corner cases where eg. dark colours would display
as bright pink.
yamt pushed a commit to yamt/netbsd-src that referenced this pull request Mar 28, 2026
netbsd-srcmastr pushed a commit that referenced this pull request Jul 26, 2026
	sys/arch/evbppc/wii/dev/wiifb.c: revision 1.8
	sys/arch/evbppc/wii/dev/gxreg.h: revision 1.1
	sys/arch/evbppc/include/wii.h: revision 1.11
	sys/arch/evbppc/wii/machdep.c: revision 1.11
	sys/arch/evbppc/wii/dev/viio.h: file removal

wii: Use GX to accelerate RGB to YUY2 conversion.

Allocate a 640x480x32 RGB buffer in memory, and when not in EMUL mode,
use the CPU to copy from the RGB buffer to EFB (taking advantage of the
pixel engine to do RGB -> YUY2 for us) and then use the GX to copy from
EFB to XFB.

Not only does this improve performance significantly in X, it also results
in better colour reproduction. The previous software method employed by
xf86-video-wsfb had some corner cases where eg. dark colours would display
as bright pink.
netbsd-srcmastr pushed a commit that referenced this pull request Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.