From ca28978995c943727da16b62fed2c3d23fd1448a Mon Sep 17 00:00:00 2001 From: Nexoniarz Date: Mon, 14 Sep 2026 22:52:20 +0200 Subject: [PATCH] dma: the 4 MB cap was the refusal, not the arena vgpud: wanted 7365 KB of DMA memory for a 1920x982 framebuffer vgpud: not enough contiguous DMA memory schinitd: restarting os.schos.vgpud (and again, and again, until schinit: supervisor stopped answering schinit replaced schinitd) Every number in that diagnostic was about the arena, and the arena had 61 MB free. The refusal came from do_dma()'s per-request cap, which was 4 MB -- exactly one framebuffer at 1280x800, chosen when that was the only thing anybody asked for. virtio-gpu's display size is the size of the **host's window**. Drag it out and the guest is asked for 1920x982x4 in one contiguous piece, and no amount of free arena helps. 32 MB now, which is 2560x1600 with room over. The cap is not there to ration memory -- the arena does that and says so properly -- it is there so a wild size cannot ask for everything by accident. DMA_ARENA_BYTES 16 -> 64 MB alongside, for the reason its own comment already gave and then got the arithmetic wrong on: either side of the handover *both* drivers hold a framebuffer, so the requirement is two at the largest mode, not one. 16 MB is two at 1280x800 and nothing bigger. Verified booting at 256M and 512M as well as 1G. And vgpud no longer dies when it cannot have what it asked for. It claims the framebuffer before creating the resource, and if the full size cannot be had it steps down by thirds to 640x480 before giving up -- and gives up with SCHINIT_EXIT_NORESTART rather than die(), because a driver that will fail identically on every restart should not be restarted. That loop is what made schinit replace the supervisor mid-boot. vgpud: the display is 1920x1080 but only 1280x720 of DMA could be had is what it says when it steps down. With the cap raised it does not need to: 1920x1080 now allocates its full 8100 KB, 0 restarts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VQ7Qx8E1q1M1bbCq8QLHS2 --- cmd/vgpud/main.c | 84 ++++++++++++++++++++++++++---------- docs/architecture/devices.md | 18 ++++++++ include/dma.h | 34 ++++++++++----- kernel/dev/pciuser.c | 19 +++++++- 4 files changed, 121 insertions(+), 34 deletions(-) diff --git a/cmd/vgpud/main.c b/cmd/vgpud/main.c index 9b9c1ce..f5822c0 100644 --- a/cmd/vgpud/main.c +++ b/cmd/vgpud/main.c @@ -418,6 +418,66 @@ bring_up(void) vputs("vgpud: display is "); vputn(width); vputs("x"); vputn(height); vputs("\n"); + // The framebuffer is claimed **before** the resource is created, because + // the resource has to be created at a size we can actually back. + // + // virtio-gpu's display size is the size of the host's window, which the + // user drags to whatever they like -- and every pixel of it is four bytes + // of contiguous DMA that has to be found twice during the handover, once + // for the kernel driver and once for this one. A big enough window does + // not fit, and what that used to look like was not a message, it was a + // loop: + // + // vgpud: wanted 7365 KB of DMA memory for a 1920x982 framebuffer + // schinitd: restarting os.schos.vgpud + // vgpud: wanted 7365 KB ... (and again, and again, until + // schinit: supervisor stopped answering schinit replaced schinitd) + // + // So: ask for what the display says, and if that cannot be had, ask for + // less. A smaller scanout on a larger display is a worse picture; a driver + // that will not start is no picture and a supervisor fighting itself. + { + struct pciuser_dma d; + int w = width, h = height, tries; + + for (tries = 0; ; tries++) { + d.size = (unsigned long long)w * h * 4; + if (ioctl(pcifd, PCIIOC_DMA, &d) >= 0) + break; + + // Two thirds each time, on an eight-pixel grid. Not halving: + // halving overshoots badly, and the arena is usually short by a + // little rather than by a lot. + w = ((w * 2 / 3) + 7) & ~7; + h = ((h * 2 / 3) + 7) & ~7; + if (w < 640 || h < 480) { + puts("vgpud: wanted "); putn(((long long)width * height * 4) / 1024); + puts(" KB of DMA memory for a "); putn(width); puts("x"); + putn(height); puts(" framebuffer, and nothing down to 640x480\n"); + puts("vgpud: `schdbg dma` shows the arena -- this is " + "DMA_ARENA_BYTES, not fragmentation\n"); + // Not die(): a died driver is restarted, and restarting will + // fail identically for ever. The kernel keeps the VBE console, + // which is a working screen. + nothing_to_do("no room for a framebuffer; the console keeps the " + "VBE framebuffer"); + } + } + + if (tries) { + puts("vgpud: the display is "); putn(width); puts("x"); putn(height); + puts(" but only "); putn(w); puts("x"); putn(h); + puts(" of DMA could be had; using that\n"); + width = w; + height = h; + } + fb_uaddr = d.uaddr; + fb_paddr = d.paddr; + } + fb_bytes = (unsigned int)(width * height * 4); + max_w = width; + max_h = height; + // A 2D resource the size of the screen. struct virtio_gpu_resource_create_2d *c = (struct virtio_gpu_resource_create_2d *)cmdbuf; @@ -429,29 +489,7 @@ bring_up(void) if (docmd(sizeof(*c), sizeof(struct virtio_gpu_ctrl_hdr)) < 0 || !resp_ok()) die("RESOURCE_CREATE_2D refused"); - // Memory behind it. This is the framebuffer: the process writes here and - // the device reads it, which is the entire reason PCIIOC_DMA reports a - // physical address. - fb_bytes = (unsigned int)(width * height * 4); - - struct pciuser_dma d; - d.size = fb_bytes; - if (ioctl(pcifd, PCIIOC_DMA, &d) < 0) { - // The kernel has already printed how much is free and how big the - // largest piece is, which is the part that says whether this is a full - // arena or a fragmented one. All this adds is what was being asked - // for, and the resolution that asked for it -- because the usual cause - // is a mode nobody realised was this expensive. - puts("vgpud: wanted "); putn(fb_bytes / 1024); - puts(" KB of DMA memory for a "); putn(width); puts("x"); putn(height); - puts(" framebuffer\n"); - puts("vgpud: `schdbg dma` shows the arena; a smaller mode needs less\n"); - die("not enough contiguous DMA memory"); - } - fb_uaddr = d.uaddr; - fb_paddr = d.paddr; - max_w = width; - max_h = height; + // The framebuffer was claimed above, before the resource was created. struct virtio_gpu_resource_attach_backing *a = (struct virtio_gpu_resource_attach_backing *)cmdbuf; diff --git a/docs/architecture/devices.md b/docs/architecture/devices.md index d82513c..a691ed6 100644 --- a/docs/architecture/devices.md +++ b/docs/architecture/devices.md @@ -102,6 +102,24 @@ a framebuffer wanting 9 MB then cannot be had on an arena three quarters empty. Best fit puts each request in the tightest extent that holds it and leaves the large ones whole for the requests with no alternative. +**One request is capped at 32 MB**, and the cap is not there to ration memory — +the arena does that, and says so properly. It is there so a wild `size` from a +process cannot ask for the whole arena by accident. It was 4 MB, which is one +framebuffer at 1280x800, chosen when that was the only thing anyone asked for. + +That is worth recording because it produced a failure that pointed somewhere +else entirely. virtio-gpu's display size is the size of the *host's window*, so +a user who drags it out to 1920x1080 asks for 8100 KB in one piece — refused by +the cap, on an arena with 61 MB free: + +``` +vgpud: wanted 7365 KB of DMA memory for a 1920x982 framebuffer +... +65536 KB total, 61847 KB free in 2 extent(s) +``` + +Every number in the diagnostic was about the arena, and the arena was fine. + A failed allocation reports both numbers for the same reason, because they point at different fixes — a fragmented arena is cured by whoever holds the block in the middle letting go, a full one by `DMA_ARENA_BYTES`: diff --git a/include/dma.h b/include/dma.h index a410018..a4b5e54 100644 --- a/include/dma.h +++ b/include/dma.h @@ -21,18 +21,32 @@ // will happily reorder stores to a ring, so a driver still needs a barrier // between filling a descriptor and telling the device about it. -// 16 MB, not 4. +// 64 MB, and the arithmetic is worth writing down because it has been got +// wrong twice. // -// 4 MB was one framebuffer at 1280x800, and that was fine while exactly one -// driver wanted one. It stopped being fine the day a display driver could be -// handed to userspace (docs/plan/hybrid.md): for the moment either side of the -// handover both drivers exist, and the incoming one has to allocate its -// framebuffer before the outgoing one is gone. +// 4 MB was one framebuffer at 1280x800, which was fine while exactly one driver +// wanted one. It stopped being fine the day a display driver could be handed to +// userspace (docs/plan/hybrid.md): either side of that handover **both drivers +// exist**, and the incoming one has to allocate its framebuffer before the +// outgoing one lets go of its own. So the real requirement is two framebuffers +// at once, not one. // -// It is carved off the top of physical memory at boot and never grows, so the -// cost of being wrong in this direction is 12 MB and the cost of being wrong -// in the other is a driver that cannot start. -#define DMA_ARENA_BYTES (16*1024*1024) +// 16 MB was chosen for two at 1280x800 (4 MB each) and that is what it holds. +// It does not hold two of anything larger, and virtio-gpu's size is **the size +// of the host's window**, which the user resizes. A 1920x982 window is 7365 KB +// a copy, so the handover needs 14.4 MB of a 16 MB arena and then fails on the +// rings and the alignment: +// +// vgpud: wanted 7365 KB of DMA memory for a 1920x982 framebuffer +// vgpud: not enough contiguous DMA memory +// schinitd: restarting os.schos.vgpud (and again, and again) +// +// 64 MB is two framebuffers at 2560x1440 with room to spare, which covers any +// window somebody is likely to drag out on a desktop. It is carved off the top +// of physical memory at boot and never grows, so it is 64 MB this kernel cannot +// use for anything else -- 6% of the 1 GB this is usually run with, against a +// display driver that otherwise cannot start at all. +#define DMA_ARENA_BYTES (64*1024*1024) void dma_init(void *vstart, void *vend); diff --git a/kernel/dev/pciuser.c b/kernel/dev/pciuser.c index ff07801..0045d62 100644 --- a/kernel/dev/pciuser.c +++ b/kernel/dev/pciuser.c @@ -589,8 +589,25 @@ do_dma(uint64 argp) if (!req) return -EFAULT; + // The cap on one request, and it is the thing that actually said no. + // + // It was 4 MB, which is one framebuffer at 1280x800 and was chosen when that + // was the only thing anybody asked for. virtio-gpu's display size is the size + // of the host's *window*, though, and a user who drags that window out to + // 1920x1080 is asking for 8100 KB in one piece. The refusal came from here + // and not from the arena, which is why it kept happening on an arena with + // 61 MB free: + // + // vgpud: wanted 7365 KB of DMA memory for a 1920x982 framebuffer + // ... + // 65536 KB total, 61847 KB free in 2 extent(s) + // + // 32 MB is a 2560x1600 framebuffer with room over. The cap is not there to + // ration memory -- the arena does that, and says so properly when it runs + // out -- it is there so that a wild `size` from a process cannot ask for the + // whole arena by accident. uint64 size = PGROUNDUP(req->size); - if (size == 0 || size > 4 * 1024 * 1024) + if (size == 0 || size > 32 * 1024 * 1024) return -EINVAL; struct proc *p = myproc();