diff --git a/Makefile b/Makefile index ebb4c75..e7be12c 100644 --- a/Makefile +++ b/Makefile @@ -107,7 +107,7 @@ KERN_NOENTRY = $(filter-out $(BUILD)/kernel/arch/x86_64/entry.o, $(KERNOBJS)) # Third-party software does NOT go here — see ports/. # ----------------------------------------------------------------------- ULIB = $(addprefix $(BUILD)/lib/, start.o glue.o usys.o) -CMDS = init schinit schinitd fbinfo shmtest socktest inputtest nettest reflink schattr ping chvt pcitest +CMDS = init schinit schinitd fbinfo shmtest socktest inputtest nettest reflink schattr ping chvt pcitest vgpud CMDBINS = $(addprefix $(BUILD)/cmd/, $(CMDS)) # ----------------------------------------------------------------------- diff --git a/cmd/vgpud/main.c b/cmd/vgpud/main.c new file mode 100644 index 0000000..f6a43a4 --- /dev/null +++ b/cmd/vgpud/main.c @@ -0,0 +1,551 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// vgpud -- the virtio-gpu driver, outside the kernel. +// +// docs/plan/hybrid.md's first eviction. The same device the kernel drives in +// kernel/drivers/gpu/virtio_gpu/, driven instead by a process that asks the +// kernel for four things and nothing else: +// +// PCIIOC_CLAIM take the device (the kernel driver is asked to let go) +// PCIIOC_CFG read config space, to find the vendor capabilities +// mmap(BAR) reach the registers +// PCIIOC_DMA memory the device can be pointed at +// +// The point is not that this is better than the in-kernel driver -- it is the +// same code in a worse language for the job. The point is that it is a real +// driver rather than a test program, so the interface has something to be +// judged by, and what it asks for is what a foreign driver would ask for too. +// +// Note what is *not* here: no syscall knows this is a GPU. The kernel handed +// over a PCI device and this decided it was a GPU, which is the property that +// makes the interface general rather than a hole shaped like one thing. + +#include "types.h" +#include "user.h" +#include "fcntl.h" +#include "posix.h" +#include "pciuser.h" +#include "virtio.h" + +// --- small output, since lib/ has no printf --------------------------------- + +static void +puts(const char *s) +{ + int n = 0; + while (s[n]) n++; + write(1, (void *)s, n); +} + +static void +putn(long long v) +{ + char b[24]; + int i = 0; + if (v < 0) { write(1, "-", 1); v = -v; } + if (v == 0) { write(1, "0", 1); return; } + while (v > 0) { b[i++] = '0' + (int)(v % 10); v /= 10; } + while (i-- > 0) write(1, &b[i], 1); +} + +static void +puthex(unsigned long long v, int digits) +{ + static const char h[] = "0123456789abcdef"; + char b[17]; + for (int i = digits - 1; i >= 0; i--) { b[i] = h[v & 0xF]; v >>= 4; } + write(1, b, digits); +} + +static void +die(const char *why) +{ + puts("vgpud: "); puts(why); puts("\n"); + exit(); +} + +// --- register access -------------------------------------------------------- + +static inline unsigned char r8 (volatile unsigned char *p, int o) { return p[o]; } +static inline void w8 (volatile unsigned char *p, int o, unsigned char v) { p[o] = v; } +static inline unsigned short r16(volatile unsigned char *p, int o) { return *(volatile unsigned short *)(p + o); } +static inline void w16(volatile unsigned char *p, int o, unsigned short v) { *(volatile unsigned short *)(p + o) = v; } +static inline unsigned int r32(volatile unsigned char *p, int o) { return *(volatile unsigned int *)(p + o); } +static inline void w32(volatile unsigned char *p, int o, unsigned int v) { *(volatile unsigned int *)(p + o) = v; } +static inline void w64(volatile unsigned char *p, int o, unsigned long long v) +{ + // Two 32-bit writes, low half first. The virtio spec requires it and the + // device latches on the high write; a single 64-bit store is a different + // bus transaction and some implementations simply drop it. + w32(p, o, (unsigned int)v); + w32(p, o + 4, (unsigned int)(v >> 32)); +} + +// --- the device ------------------------------------------------------------- + +static int pcifd; +static int index_; + +static volatile unsigned char *common; +static volatile unsigned char *notify; +static volatile unsigned char *isr; +static unsigned int notify_mult; + +static struct pciuser_info info; + +static unsigned int +cfg32(unsigned int off) +{ + struct pciuser_cfg c; + c.index = (unsigned int)index_; + c.offset = off & ~3u; + c.value = 0; + if (ioctl(pcifd, PCIIOC_CFG, &c) < 0) + die("config read failed"); + return c.value; +} + +static unsigned char +cfg8(unsigned int off) +{ + return (unsigned char)(cfg32(off) >> ((off & 3) * 8)); +} + +// Find a virtio configuration structure. +// +// A virtio 1.0 device does not put its registers at a fixed offset. It walks +// you there: a vendor capability in config space names which BAR, and how far +// into it, each structure lives. So a driver that cannot read config space +// cannot find its own registers -- which is why PCIIOC_CFG exists, and why the +// test program that came before this never needed it. +static volatile unsigned char * +find_cfg(int want, unsigned int *out_mult) +{ + unsigned char off = cfg8(0x34) & 0xFC; + + while (off) { + unsigned char id = cfg8(off); + unsigned char next = cfg8(off + 1); + + if (id == PCI_CAP_ID_VNDR) { + unsigned char type = cfg8(off + 3); + unsigned char bar = cfg8(off + 4); + unsigned int o = cfg32(off + 8); + unsigned int len = cfg32(off + 12); + + if (type == want && bar < PCIUSER_MAXBAR && info.bar[bar].base) { + if (out_mult) + *out_mult = cfg32(off + 16); + + // Map from the start of the BAR, not from the capability's + // offset: two capabilities usually live in one BAR, and + // mapping each separately would hand out two mappings of + // overlapping pages. + unsigned long long want_len = (unsigned long long)o + len; + if (want_len < 4096) want_len = 4096; + if (want_len > info.bar[bar].size) want_len = info.bar[bar].size; + + volatile unsigned char *p = + (volatile unsigned char *)mmap(0, want_len, + PROT_READ | PROT_WRITE, + MAP_SHARED, pcifd, + info.bar[bar].base); + if (p == 0 || (long)p == -1) + return 0; + return p + o; + } + } + off = next & 0xFC; + } + return 0; +} + +// --- the queue -------------------------------------------------------------- + +#define CTRL_QUEUE 0 + +static struct vring_desc *desc; +static struct vring_avail *avail; +static struct vring_used *used; +static unsigned long long ring_phys; +static unsigned short qsize, notify_off, last_used, next_desc; + +static void *cmdbuf, *respbuf; +static unsigned long long cmdphys, respphys; + +static void +queue_setup(void) +{ + w16(common, VCOMMON_Q_SELECT, CTRL_QUEUE); + qsize = r16(common, VCOMMON_Q_SIZE); + if (qsize == 0) + die("control queue has no slots"); + if (qsize > 64) + qsize = 64; // more than one command in flight is not + // something this driver has any use for + w16(common, VCOMMON_Q_SIZE, qsize); + notify_off = r16(common, VCOMMON_Q_NOTIFY_OFF); + + // One contiguous block for the three rings, laid out the way the spec + // requires: descriptors, then available, then used, each aligned. The + // device reads all three by physical address, so they have to come from + // the kernel's DMA arena rather than from anything this process could + // allocate for itself. + unsigned int desc_sz = 16 * qsize; + unsigned int avail_sz = 6 + 2 * qsize; + + struct pciuser_dma d; + d.size = 16384; + if (ioctl(pcifd, PCIIOC_DMA, &d) < 0) + die("no DMA memory for the queue"); + + unsigned long long base = d.uaddr; + unsigned long long basep = d.paddr; + + unsigned int off_avail = (desc_sz + 1) & ~1u; + unsigned int off_used = (off_avail + avail_sz + 3) & ~3u; + + desc = (struct vring_desc *)(base); + avail = (struct vring_avail *)(base + off_avail); + used = (struct vring_used *)(base + off_used); + ring_phys = basep; + + for (unsigned int i = 0; i < 16384; i++) + ((volatile unsigned char *)base)[i] = 0; + + // Do not interrupt us. docmd() polls the used ring, so an interrupt is + // not merely unwanted, it is dangerous: this process holds the device but + // has no handler registered, and the line is shared with the e1000. A + // level-triggered line nobody acknowledges re-fires for ever -- 77,000 + // times a second, which is what the first working run of this driver did + // before this line existed. The storm detector in kernel/dev/irq.c named + // the line and the neighbour it was drowning. + avail->flags = VRING_AVAIL_F_NO_INTERRUPT; + __sync_synchronize(); + + w64(common, VCOMMON_Q_DESC, basep); + w64(common, VCOMMON_Q_DRIVER, basep + off_avail); + w64(common, VCOMMON_Q_DEVICE, basep + off_used); + w16(common, VCOMMON_Q_ENABLE, 1); + + // Command and response buffers, also from the arena for the same reason. + struct pciuser_dma b; + b.size = 8192; + if (ioctl(pcifd, PCIIOC_DMA, &b) < 0) + die("no DMA memory for command buffers"); + cmdbuf = (void *)b.uaddr; + cmdphys = b.paddr; + respbuf = (void *)(b.uaddr + 4096); + respphys = b.paddr + 4096; +} + +// Submit one command and wait for the answer. +// +// Polled, not interrupt-driven. This driver issues a handful of commands at +// startup and one flush per frame, and a poll on a used-ring index that the +// device updates by DMA is both simpler and lower-latency than waking a +// process. The interrupt path exists (PCIIOC_IRQ) and belongs in whatever +// wants to sleep between frames. +static int +docmd(unsigned int cmdlen, unsigned int resplen) +{ + unsigned short head = next_desc; + + desc[head].addr = cmdphys; + desc[head].len = cmdlen; + desc[head].flags = VRING_DESC_F_NEXT; + desc[head].next = (unsigned short)(head + 1); + + desc[head + 1].addr = respphys; + desc[head + 1].len = resplen; + desc[head + 1].flags = VRING_DESC_F_WRITE; + desc[head + 1].next = 0; + + avail->ring[avail->idx % qsize] = head; + __sync_synchronize(); + avail->idx++; + __sync_synchronize(); + + *(volatile unsigned short *)(notify + notify_off * notify_mult) = CTRL_QUEUE; + + // Spin with a bound. A device that never answers is a bug somewhere, and + // hanging for ever is a worse way to report it than saying so. + for (int spin = 0; spin < 100000000; spin++) { + __sync_synchronize(); + if (used->idx != last_used) { + last_used = used->idx; + next_desc = (unsigned short)((next_desc + 2) % qsize); + return 0; + } + } + return -1; +} + +static void +hdr_init(void *p, unsigned int type) +{ + struct virtio_gpu_ctrl_hdr *h = (struct virtio_gpu_ctrl_hdr *)p; + for (unsigned int i = 0; i < sizeof(*h); i++) ((unsigned char *)h)[i] = 0; + h->type = type; +} + +static int +resp_ok(void) +{ + struct virtio_gpu_ctrl_hdr *h = (struct virtio_gpu_ctrl_hdr *)respbuf; + return h->type == VIRTIO_GPU_RESP_OK_NODATA || + h->type == VIRTIO_GPU_RESP_OK_DISPLAY_INFO; +} + +// --- bringing the device up ------------------------------------------------- + +static int width, height; +static unsigned long long fb_uaddr, fb_paddr; +static unsigned int fb_bytes; + +#define RESOURCE_ID 1 + +static void +bring_up(void) +{ + // Reset, then the status handshake in the order the spec sets out. A + // device that finds the sequence wrong fails the feature step rather than + // telling you which line was at fault, so it is written out plainly. + w8(common, VCOMMON_STATUS, 0); + while (r8(common, VCOMMON_STATUS) != 0) + ; + w8(common, VCOMMON_STATUS, VSTATUS_ACK); + w8(common, VCOMMON_STATUS, VSTATUS_ACK | VSTATUS_DRIVER); + + w32(common, VCOMMON_DFEAT_SEL, 1); + unsigned int hi = r32(common, VCOMMON_DFEAT); + if (!(hi & (1u << (VIRTIO_F_VERSION_1 - 32)))) { + w8(common, VCOMMON_STATUS, VSTATUS_FAILED); + die("device is not virtio 1.0"); + } + + w32(common, VCOMMON_GFEAT_SEL, 0); + w32(common, VCOMMON_GFEAT, 0); + w32(common, VCOMMON_GFEAT_SEL, 1); + w32(common, VCOMMON_GFEAT, 1u << (VIRTIO_F_VERSION_1 - 32)); + + w8(common, VCOMMON_STATUS, VSTATUS_ACK | VSTATUS_DRIVER | VSTATUS_FEATURES_OK); + if (!(r8(common, VCOMMON_STATUS) & VSTATUS_FEATURES_OK)) { + w8(common, VCOMMON_STATUS, VSTATUS_FAILED); + die("device refused the feature set"); + } + + queue_setup(); + + w8(common, VCOMMON_STATUS, + VSTATUS_ACK | VSTATUS_DRIVER | VSTATUS_FEATURES_OK | VSTATUS_DRIVER_OK); + + // What size is the screen? + hdr_init(cmdbuf, VIRTIO_GPU_CMD_GET_DISPLAY_INFO); + if (docmd(sizeof(struct virtio_gpu_ctrl_hdr), + sizeof(struct virtio_gpu_resp_display_info)) < 0) + die("the device never answered GET_DISPLAY_INFO"); + if (!resp_ok()) + die("GET_DISPLAY_INFO refused"); + + struct virtio_gpu_resp_display_info *di = + (struct virtio_gpu_resp_display_info *)respbuf; + width = (int)di->pmodes[0].r.width; + height = (int)di->pmodes[0].r.height; + if (width <= 0 || height <= 0) + die("the device reports no display"); + + puts("vgpud: display is "); putn(width); puts("x"); putn(height); puts("\n"); + + // A 2D resource the size of the screen. + struct virtio_gpu_resource_create_2d *c = + (struct virtio_gpu_resource_create_2d *)cmdbuf; + hdr_init(c, VIRTIO_GPU_CMD_RESOURCE_CREATE_2D); + c->resource_id = RESOURCE_ID; + c->format = VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM; + c->width = (unsigned int)width; + c->height = (unsigned int)height; + 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) { + puts("vgpud: wanted "); putn(fb_bytes / 1024); + puts(" KB of DMA memory for the framebuffer\n"); + die("not enough DMA memory -- see DMA_ARENA_BYTES"); + } + fb_uaddr = d.uaddr; + fb_paddr = d.paddr; + + struct virtio_gpu_resource_attach_backing *a = + (struct virtio_gpu_resource_attach_backing *)cmdbuf; + hdr_init(a, VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING); + a->resource_id = RESOURCE_ID; + a->nr_entries = 1; + // The entries follow the struct rather than being in it: the count is + // variable, so the command is a header with an array glued on the end. + struct virtio_gpu_mem_entry *e = (struct virtio_gpu_mem_entry *)(a + 1); + e->addr = fb_paddr; + e->length = fb_bytes; + e->padding = 0; + if (docmd(sizeof(*a) + sizeof(*e), sizeof(struct virtio_gpu_ctrl_hdr)) < 0 || + !resp_ok()) + die("RESOURCE_ATTACH_BACKING refused"); + + // Put it on the screen. + struct virtio_gpu_set_scanout *so = (struct virtio_gpu_set_scanout *)cmdbuf; + hdr_init(so, VIRTIO_GPU_CMD_SET_SCANOUT); + so->r.x = so->r.y = 0; + so->r.width = (unsigned int)width; + so->r.height = (unsigned int)height; + so->scanout_id = 0; + so->resource_id = RESOURCE_ID; + if (docmd(sizeof(*so), sizeof(struct virtio_gpu_ctrl_hdr)) < 0 || !resp_ok()) + die("SET_SCANOUT refused"); + + puts("vgpud: scanout is ours, framebuffer at physical 0x"); + puthex(fb_paddr, 16); puts("\n"); +} + +// Push what has been drawn to the host. Two commands, and both are needed: +// the transfer copies from the backing into the device's copy of the resource, +// the flush tells it to put that on screen. +static void +present(void) +{ + struct virtio_gpu_transfer_to_host_2d *t = + (struct virtio_gpu_transfer_to_host_2d *)cmdbuf; + hdr_init(t, VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D); + t->r.x = t->r.y = 0; + t->r.width = (unsigned int)width; + t->r.height = (unsigned int)height; + t->offset = 0; + t->resource_id = RESOURCE_ID; + t->padding = 0; + if (docmd(sizeof(*t), sizeof(struct virtio_gpu_ctrl_hdr)) < 0) + die("TRANSFER_TO_HOST_2D never answered"); + + struct virtio_gpu_resource_flush *f = + (struct virtio_gpu_resource_flush *)cmdbuf; + hdr_init(f, VIRTIO_GPU_CMD_RESOURCE_FLUSH); + f->r.x = f->r.y = 0; + f->r.width = (unsigned int)width; + f->r.height = (unsigned int)height; + f->resource_id = RESOURCE_ID; + f->padding = 0; + if (docmd(sizeof(*f), sizeof(struct virtio_gpu_ctrl_hdr)) < 0) + die("RESOURCE_FLUSH never answered"); +} + +// --- something to look at --------------------------------------------------- + +// Not a compositor. A pattern that is obviously deliberate, so a screenshot +// answers "did a userspace process put this here" without ambiguity -- a black +// screen and a working screen look identical if all you draw is black. +static void +draw_test_pattern(void) +{ + volatile unsigned int *fb = (volatile unsigned int *)fb_uaddr; + + for (int y = 0; y < height; y++) { + for (int x = 0; x < width; x++) { + unsigned int c; + int band = (y * 8) / height; + switch (band) { + case 0: c = 0x0C0E13; break; /* SCH_BG */ + case 1: c = 0x7C5CFF; break; /* SCH_ACCENT */ + case 2: c = 0x4B1D9A; break; /* PSOGM purple */ + case 3: c = (unsigned int)((x * 255 / width) * 0x010101); break; + case 4: c = (unsigned int)(x * 255 / width) << 16; break; + case 5: c = (unsigned int)(x * 255 / width) << 8; break; + case 6: c = (unsigned int)(x * 255 / width); break; + default: + // A checkerboard, so a stride bug is visible as a shear rather + // than as a plausible-looking gradient. + c = (((x / 16) + (y / 16)) & 1) ? 0xE6E8EC : 0x0C0E13; + break; + } + fb[y * width + x] = c; + } + } +} + +// --- main ------------------------------------------------------------------- + +int +main(int argc, char *argv[]) +{ + pcifd = open("/dev/pci", O_RDWR); + if (pcifd < 0) + die("cannot open /dev/pci"); + + int count = 0; + if (ioctl(pcifd, PCIIOC_COUNT, &count) < 0) + die("PCIIOC_COUNT failed"); + + // Find it by what it is, not by where it is. A driver that takes a bus + // address on the command line is one nobody can start automatically. + index_ = -1; + for (int i = 0; i < count; i++) { + info.index = (unsigned int)i; + if (ioctl(pcifd, PCIIOC_INFO, &info) < 0) + continue; + if (info.vendor == VIRTIO_VENDOR && info.device == VIRTIO_DEV_GPU) { + index_ = i; + break; + } + } + if (index_ < 0) + die("no virtio-gpu on this machine"); + + puts("vgpud: virtio-gpu at "); puthex(info.bus, 2); puts(":"); + puthex(info.slot, 2); puts("."); puthex(info.func, 1); + if (info.driver[0]) { puts(", currently held by "); puts(info.driver); } + puts("\n"); + + // The kernel driver is asked to let go here. It puts the VBE framebuffer + // back on screen before returning, so there is never a moment with no + // display -- see vgpu_release() and vga_fb_restore(). + if (ioctl(pcifd, PCIIOC_CLAIM, &index_) < 0) + die("claim refused -- the kernel driver would not release it"); + puts("vgpud: claimed\n"); + + // The info is re-read because claiming is what enables memory space and + // bus mastering, and a BAR read before that can legitimately be zero. + info.index = (unsigned int)index_; + if (ioctl(pcifd, PCIIOC_INFO, &info) < 0) + die("PCIIOC_INFO failed after claim"); + + common = find_cfg(VIRTIO_PCI_CAP_COMMON_CFG, 0); + notify = find_cfg(VIRTIO_PCI_CAP_NOTIFY_CFG, ¬ify_mult); + isr = find_cfg(VIRTIO_PCI_CAP_ISR_CFG, 0); + if (!common || !notify) + die("no modern virtio capabilities -- not a 1.0 device"); + + bring_up(); + draw_test_pattern(); + present(); + + puts("vgpud: drew a test pattern from userspace\n"); + + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'x') { + // Exit without releasing, to show what a crashed driver leaves behind: + // the kernel takes the device back and the screen is whatever the + // device was last told to scan out. + puts("vgpud: exiting without cleaning up (simulated crash)\n"); + exit(); + } + + // Nothing to serve yet -- the compositor is what will connect here, over + // mechanism 1, which does not exist. Holding the device is the honest + // thing to do until then: releasing it would put the console back and make + // it look like this never worked. + for (;;) + sleep(100); +} diff --git a/docs/plan/hybrid.md b/docs/plan/hybrid.md index 5829d16..5504853 100644 --- a/docs/plan/hybrid.md +++ b/docs/plan/hybrid.md @@ -102,6 +102,61 @@ Five ticks lost in seven seconds, which is the ordinary background. The next driver to claim the device re-arms the line, so masked is safe without being permanent -- also verified. +## The first eviction happened + +`cmd/vgpud` is the virtio-gpu driver, outside the kernel. It finds the device +by vendor and device id, asks for it, and the kernel driver hands it over: + +``` +vgpud: virtio-gpu at 00:03.0, currently held by virtio-gpu +virtio-gpu: released to userspace; screen is the VBE framebuffer +vgpud: claimed +vgpud: display is 1280x800 +vgpud: scanout is ours, framebuffer at physical 0x3eceb000 +vgpud: drew a test pattern from userspace +``` + +Full virtio 1.0 bring-up from a process: reset, feature negotiation, virtqueue +in DMA memory, GET_DISPLAY_INFO, RESOURCE_CREATE_2D, ATTACH_BACKING, +SET_SCANOUT, and TRANSFER/FLUSH to present. The pattern it draws is on the +virtio-gpu head while the console carries on in VBE on the other one -- which +is the fallback rule working rather than being described. + +**No syscall knows this is a GPU.** The kernel handed over a PCI device; the +process decided what it was. That is the property that makes the interface +general, and it is why the evictions come before anything foreign. + +### What the first real driver asked for that a test program had not + +- **PCI configuration space** (`PCIIOC_CFG`). A virtio device says where its + registers are in *vendor capabilities*, which live in config space -- so a + driver that cannot read it cannot find its own registers. Anything modern is + discovered the same way. +- **A way for the incumbent to let go.** `struct pci_driver` gained a + `release` hook. A driver without one keeps its device, which is always safe + and, for anything on the path to a shell, correct. +- **The outgoing driver freeing its DMA.** Releasing the device while keeping + its 4 MB framebuffer is half a handover: the incoming driver asks for one and + is told there is no room. Found by doing it. +- **A bigger arena.** 4 MB was exactly one framebuffer at this resolution, and + for the moment either side of a handover there are two. 16 MB now. + +### And one bug it reproduced immediately + +The first working run raised this: + +``` +irq 11: 77780 interrupts/second -- a device on this line is not acknowledging it. +irq 11: handler 'e1000' is attached +``` + +`vgpud` polls the used ring, so it registered no handler -- and a virtio device +interrupts on every completion. A level-triggered line nobody acknowledges +re-fires for ever, drowning the e1000 that shares it. The fix is one line, +`VRING_AVAIL_F_NO_INTERRUPT` before the queue is enabled, and the value of it +is that the storm detector built for history/kernel.md section 13 named the +line, the rate and the neighbour without anyone having to look. + **The control channel (mechanism 1) is what remains.** ### One thing the interface is deliberately not diff --git a/include/defs.h b/include/defs.h index 4557676..e2cd78b 100644 --- a/include/defs.h +++ b/include/defs.h @@ -608,6 +608,7 @@ void platform_register_devices(uint64 fb_addr); // from registry.c; everything else draws through include/gfx.h. void vga_fb_init(uint64 phys_addr); uint64 vga_fb_phys(void); +void vga_fb_restore(void); uint64 vga_fb_size(void); void e820_init(void); diff --git a/include/dma.h b/include/dma.h index 1a40de9..f15c3e0 100644 --- a/include/dma.h +++ b/include/dma.h @@ -21,7 +21,18 @@ // will happily reorder stores to a ring, so a driver still needs a barrier // between filling a descriptor and telling the device about it. -#define DMA_ARENA_BYTES (4*1024*1024) +// 16 MB, not 4. +// +// 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. +// +// 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) void dma_init(void *vstart, void *vend); diff --git a/include/pci.h b/include/pci.h index b34f67a..77e725f 100644 --- a/include/pci.h +++ b/include/pci.h @@ -71,6 +71,9 @@ struct pci_dev { uint8 header_type; struct pci_bar bar[6]; const char *driver; // name of whatever claimed it, or 0 + // The driver itself, so it can be asked to release the device. The name + // alone cannot be called. + const struct pci_driver *drv; }; // One row of a driver's match table. Terminate the table with a row whose @@ -116,4 +119,17 @@ struct pci_driver { // Without this the second card of the same model is either refused or // silently shares the first one's registers. void (*attached)(struct pci_dev *, struct device *); + // Give the device up, so a userspace driver can take it. + // + // docs/plan/hybrid.md is built on the kernel *handing over* a device + // rather than a process wrestling it away: two drivers writing one set of + // registers is not a subtle failure, it is hardware that appears to have + // started lying. So the transfer is the incumbent's decision, and this is + // where it is made. + // + // A driver that implements this must leave the device in a state the next + // one can start from, and must stop touching it. A driver that does not + // implement it keeps its device: refusing is always safe, and for anything + // on the path to a shell it is also correct. + int (*release)(struct pci_dev *); }; diff --git a/include/pciuser.h b/include/pciuser.h index 4a18f42..77754c3 100644 --- a/include/pciuser.h +++ b/include/pciuser.h @@ -69,6 +69,16 @@ struct pciuser_info { // A DMA buffer. The kernel allocates it from the DMA arena, maps it into the // caller, and reports the physical address the *device* must be given -- which // is the entire reason a driver cannot do this for itself with mmap. +// A dword of PCI configuration space. A virtio device says where its registers +// live in vendor capabilities, which are here and nowhere else -- so without +// this a driver cannot find its own device. +struct pciuser_cfg { + unsigned int index; // in + unsigned int offset; // in: dword-aligned, 0..0xFC + unsigned int value; // out + unsigned int pad; +}; + struct pciuser_dma { unsigned long long size; // in: bytes, rounded up to a page unsigned long long uaddr; // out: where it landed in this process @@ -80,6 +90,7 @@ struct pciuser_dma { #define PCIIOC_CLAIM 0x9002 // in: int index -- take it from the kernel #define PCIIOC_RELEASE 0x9003 // in: int index #define PCIIOC_DMA 0x9004 // struct pciuser_dma +#define PCIIOC_CFG 0x9006 // struct pciuser_cfg -- read config space #define PCIIOC_IRQ 0x9005 // in: int index -- deliver this device's // interrupts to this process diff --git a/kernel/drivers/gpu/virtio_gpu/virtio.h b/include/virtio.h similarity index 83% rename from kernel/drivers/gpu/virtio_gpu/virtio.h rename to include/virtio.h index 5cbff3f..c2ed601 100644 --- a/kernel/drivers/gpu/virtio_gpu/virtio.h +++ b/include/virtio.h @@ -2,11 +2,14 @@ // virtio 1.0 over PCI — the transport, and virtio-gpu's 2D protocol. // -// Next to the driver rather than in include/, because exactly one driver reads -// it. When a second virtio device arrives -- virtio-net and virtio-blk are the -// obvious ones -- the *transport* half of this belongs in a shared place and -// the 2D protocol stays here; splitting it before there is a second caller -// would be guessing at where the seam goes. +// It lived next to the driver while exactly one driver read it, with a note +// saying it should move when a second reader appeared. One has: cmd/vgpud is +// the same device driven from userspace (docs/plan/hybrid.md's first +// eviction), and it needs the same register offsets and the same commands. +// +// Still one file rather than transport-and-protocol split in two. Both readers +// want both halves, so a seam here would be drawn for the sake of having one. +// virtio-net or virtio-blk arriving is when that changes. // // Written from the OASIS specification (virtio 1.1, chapters 2, 4.1 and 5.7), // which is a public document with no licence on what you write from it. Same @@ -68,6 +71,16 @@ // --- split virtqueue ------------------------------------------------------- #define VRING_DESC_F_NEXT 1 + +// Set in vring_avail.flags to tell the device not to interrupt on completion. +// +// A driver that polls the used ring wants this, and wants it before the queue +// is enabled. Without it the device raises its interrupt line on every +// completed command -- and if nothing is registered to acknowledge it, a +// level-triggered PCI line stays asserted and re-fires for ever. That is the +// failure in docs/history/kernel.md section 13, and cmd/vgpud reproduced it +// exactly on its first working run. +#define VRING_AVAIL_F_NO_INTERRUPT 1 #define VRING_DESC_F_WRITE 2 // device writes it; i.e. this is our buffer struct vring_desc { diff --git a/kernel/dev/pci.c b/kernel/dev/pci.c index fb517eb..5c10af2 100644 --- a/kernel/dev/pci.c +++ b/kernel/dev/pci.c @@ -374,6 +374,7 @@ pci_bind(const struct pci_driver *drv) continue; if (drv->probe(d) == 0) { d->driver = drv->name; + d->drv = drv; // Publish it in the bus-agnostic device list. Done here rather than in // each driver's probe() so that a driver cannot forget to, and so the // list is populated in bind order -- which is the order a device diff --git a/kernel/dev/pciuser.c b/kernel/dev/pciuser.c index c0a620c..abfb4de 100644 --- a/kernel/dev/pciuser.c +++ b/kernel/dev/pciuser.c @@ -253,8 +253,18 @@ do_claim(uint64 argp) // has started lying. The eviction path (docs/plan/hybrid.md) is for the // kernel to *hand over* a device deliberately; until a driver can be told to // let go, the honest answer to "may I have the one you are using" is no. - if (d->driver) - return -EBUSY; + if (d->driver) { + // Ask it to let go. A driver with no release hook keeps its device, which + // is always safe and, for anything on the path to a shell, correct: the + // rule in docs/plan/hybrid.md is that the kernel keeps a minimal driver + // for everything needed to reach a prompt and userspace owns the good one. + if (!d->drv || !d->drv->release) + return -EBUSY; + if (d->drv->release((struct pci_dev *)d) != 0) + return -EBUSY; + ((struct pci_dev *)d)->driver = 0; + ((struct pci_dev *)d)->drv = 0; + } acquire(&pciuser_lock); if (owner[idx] && owner[idx] != myproc()) { @@ -391,6 +401,39 @@ do_irq(uint64 argp) return 0; } +// PCI configuration space. +// +// The first real driver asked for this and the test program never had to: a +// virtio device describes where its registers are in *vendor capabilities*, +// which live in config space, so a driver that cannot read config space cannot +// find its own registers. Anything modern is discovered the same way -- MSI-X +// tables, PCIe extended capabilities, power management. +// +// Reads are unrestricted for a claimed device; writes are not offered at all +// yet. Nothing needs them, and a write to the wrong offset re-programs a BAR +// or disables bus mastering under the kernel's feet. +static int +do_cfg(uint64 argp) +{ + struct pciuser_cfg *c = (struct pciuser_cfg *)user_ptr(argp, sizeof(*c)); + if (!c) + return -EFAULT; + + unsigned int idx = c->index; + if (idx >= (unsigned int)pci_count() || idx >= NPCI) + return -ENODEV; + if (owner[idx] != myproc()) + return -EACCES; + if (c->offset > 0xFC) + return -EINVAL; + + const struct pci_dev *d = pci_get((int)idx); + if (!d) + return -ENODEV; + c->value = pci_read32(d->bus, d->slot, d->func, (uint8)(c->offset & 0xFC)); + return 0; +} + int pciuser_ioctl(uint64 request, uint64 argp) { @@ -405,6 +448,7 @@ pciuser_ioctl(uint64 request, uint64 argp) case PCIIOC_RELEASE: return do_release(argp); case PCIIOC_DMA: return do_dma(argp); case PCIIOC_IRQ: return do_irq(argp); + case PCIIOC_CFG: return do_cfg(argp); default: return -ENOTTY; } } diff --git a/kernel/drivers/gpu/vga/vga_fb.c b/kernel/drivers/gpu/vga/vga_fb.c index 1c61703..b1f9b8e 100644 --- a/kernel/drivers/gpu/vga/vga_fb.c +++ b/kernel/drivers/gpu/vga/vga_fb.c @@ -214,3 +214,31 @@ const struct display_ops vga_display_ops = { .present = vga_disp_present, .wait_vblank = vga_disp_wait_vblank, }; + +// Put the VBE framebuffer back on screen. +// +// Called when a better display driver lets go of the hardware -- see +// virtio-gpu's release hook. gfx holds one scanout, last writer wins, so the +// driver that leaves has to name its replacement rather than simply detaching; +// detaching alone leaves gfx pointing at a device nobody is driving. +// +// This is the fallback rule from docs/plan/hybrid.md made real: the kernel +// keeps a minimal driver for everything on the path to a shell, so a fancy +// driver leaving degrades to a working screen instead of to no screen. +void +vga_fb_restore(void) +{ + struct fb_info info; + + if (!fb || !fb_width || !fb_height) + return; // nothing was ever set up; nothing to go back to + + info.ptr = fb; + info.phys = fb_phys; + info.size = vga_fb_size(); + info.stride = fb_stride; + info.bpp = fb_bpp; + info.width = fb_width; + info.height = fb_height; + gfx_attach_scanout(&info); +} diff --git a/kernel/drivers/gpu/virtio_gpu/virtio_gpu.c b/kernel/drivers/gpu/virtio_gpu/virtio_gpu.c index 384e0df..677107d 100644 --- a/kernel/drivers/gpu/virtio_gpu/virtio_gpu.c +++ b/kernel/drivers/gpu/virtio_gpu/virtio_gpu.c @@ -58,6 +58,7 @@ struct vgpu { // command queue with one entry is not the bottleneck in a 2D compositor. void *cmd; void *resp; + void *ring; // the three vrings, one block, for dma_free int width, height; void *fb; // the framebuffer we scan out from @@ -179,6 +180,7 @@ queue_setup(struct vgpu *g) uint32 used_sz = 6 + 8 * qsize; uint8 *mem = dma_alloc(4096 * 3, 4096); + g->ring = mem; if (!mem) return -1; memset(mem, 0, 4096 * 3); @@ -510,6 +512,62 @@ vgpu_probe(struct pci_dev *d) return 0; } +// Hand the device to a userspace driver. +// +// docs/plan/hybrid.md's first eviction, and the reason it is this device: the +// boundary already exists (struct display_ops), it has exactly one consumer, +// and there is a working fallback underneath. So letting go is safe in a way +// it would not be for the disk the root filesystem is on. +// +// What "letting go" has to mean, in order: +// +// 1. stop drawing -- put the VBE framebuffer back on screen *first*, so +// there is never a moment where gfx points at hardware nobody drives +// 2. reset the device, so the next driver starts from a known state rather +// than inheriting a configured queue it did not set up +// 3. forget the instance, so a later probe or a second claim cannot find +// half-initialised state +static int +vgpu_release(struct pci_dev *d) +{ + struct vgpu *g = 0; + + for (int i = 0; i < VGPU_MAX; i++) + if (gpus[i].ready && gpus[i].pdev == d) { g = &gpus[i]; break; } + if (!g) + return -1; + + vga_fb_restore(); + + // Status 0 is the virtio reset. The spec requires a driver to wait for the + // device to acknowledge by reading the register back as zero. + if (g->common) { + w8(g->common, VCOMMON_STATUS, 0); + for (int spin = 0; spin < 1000000 && r8(g->common, VCOMMON_STATUS); spin++) + ; + } + + if (g->isr) + irq_unregister(d->irq, vgpu_intr, g); + + // Give the arena its memory back. + // + // Releasing the *device* without releasing its memory is half a handover: + // the DMA arena is 16 MB and contiguous, a framebuffer at this resolution is + // 4 MB of it, and the process taking over needs one of its own. Keeping ours + // means the new driver asks for a framebuffer and is told there is no room, + // which is what happened the first time this was tried. + if (g->fb) { dma_free(g->fb, g->fb_bytes); g->fb = 0; } + if (g->ring) { dma_free(g->ring, 4096 * 3); g->ring = 0; } + if (g->cmd) { dma_free(g->cmd, 4096); g->cmd = 0; } + if (g->resp) { dma_free(g->resp, 4096); g->resp = 0; } + + g->ready = 0; + g->pdev = 0; + cprintf("virtio-gpu: released to userspace; screen is the VBE framebuffer\n"); + return 0; +} + static const struct pci_match virtio_gpu_match[] = { { VIRTIO_VENDOR, VIRTIO_DEV_GPU, 0, 0 }, { 0, 0, 0, 0 }, @@ -534,4 +592,5 @@ const struct pci_driver virtio_gpu_driver = { .ops = &virtio_gpu_display_ops, .probe = vgpu_probe, .attached = vgpu_attached, + .release = vgpu_release, };