hybrid: the driver is killed, and the driver comes back
H2 of docs/plan/hybrid.md -- the milestone the plan itself calls the one
that proves the whole idea, and the one that cannot be skipped.
# cat /proc/schinit
1 4 11 os.schos.vgpud
# kill 4
schinitd: restarting os.schos.vgpud
vgpud: claimed
vgpud: pid 8 drew a test pattern from userspace
The test pattern ends in a twelve-block tally of the drawing process's
pid, because a second run that redraws an identical picture is
indistinguishable from a first run that never stopped. Read off the
virtio-gpu scanout across four generations it says 4, 8, 10, 12, and the
DMA arena hands back the same physical address every time.
What it took:
* A halt the kernel can apply to a device it does not understand.
Clearing PCI_CMD_BUS_MASTER is the only one there is, and it has to
happen before the DMA goes back to the arena -- a dead driver's device
keeps reading a framebuffer the kernel is about to give to somebody
else. Claim sets the bit, release clears it; pcitest shows it in a bm
column, with a third state so a failed read is never printed as off.
* A refusal. vgpu_release() asks vga_fb_restore() whether there is a
framebuffer to put the console back on, and declines to hand over the
device if there is not. The rule that the kernel keeps a minimal
driver for everything on the path to a shell is worth writing down
only if something enforces it.
* The DMA arena not being owned by two allocators at once. This is the
bug that stopped H2: freevm() frees every mapped page below PHYSTOP
and the arena is the top 16 MB of exactly that range, so a DMA block
mapped into a driver went onto the page allocator's free list when the
driver died, while pciuser_release_all() handed the same pages back to
the arena. The restarted vgpud asked for its framebuffer, got pages
kalloc() had already given out -- including its own page tables -- and
dma_alloc()'s zeroing wiped them, so it died inside the ioctl on the
return from a syscall whose stack had stopped existing. do_dma() now
records a VMA with no_free set, the way shared memory does.
* One exit status. KeepAlive must restart a driver that crashed and must
not restart one that has looked at the machine and found its device
absent: on hardware with no virtio-gpu that was fifteen spawns in
forty-five seconds, each printing a line to the console, for ever.
SCHINIT_EXIT_NORESTART is 78, sysexits.h's EX_CONFIG. It is not a new
manifest key -- the wait status has always travelled from PID 1 to the
supervisor, and note_exit() simply stopped discarding it.
The milestone's third clause, "the compositor reattaches", is not done
and is not claimed: there is no compositor running. That is recorded in
the plan as a Schcom milestone rather than counted here.
Closes the register row "A released PCI device" in docs/plan/1.0.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQ7Qx8E1q1M1bbCq8QLHS2
This commit is contained in:
parent
c0ed23cc56
commit
d1f50c4da3
15 changed files with 442 additions and 40 deletions
|
|
@ -77,6 +77,24 @@ pci_enable(struct pci_dev *d, uint16 bits)
|
|||
pci_write32(d->bus, d->slot, d->func, PCI_COMMAND, cmd | bits);
|
||||
}
|
||||
|
||||
// The other direction, and PCI_CMD_BUS_MASTER is the reason it exists.
|
||||
//
|
||||
// Clearing that bit is the only way this kernel can stop a device it does not
|
||||
// understand from touching memory. Everything else about halting a device is
|
||||
// device-specific -- a different register on every chip -- but bus mastering is
|
||||
// in the standard config header of every PCI device ever made, and clearing it
|
||||
// means the next DMA cycle does not happen.
|
||||
//
|
||||
// That makes it the correct thing to do when a userspace driver dies: the
|
||||
// kernel is about to take its DMA buffers back, and a device still scanning out
|
||||
// of them would read whatever is put there next. See kernel/dev/pciuser.c.
|
||||
void
|
||||
pci_disable(struct pci_dev *d, uint16 bits)
|
||||
{
|
||||
uint32 cmd = pci_read32(d->bus, d->slot, d->func, PCI_COMMAND);
|
||||
pci_write32(d->bus, d->slot, d->func, PCI_COMMAND, cmd & ~(uint32)bits);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Capabilities
|
||||
//
|
||||
|
|
|
|||
|
|
@ -131,16 +131,49 @@ pciuser_mmio_ok(uint64 phys, uint64 len)
|
|||
return 0;
|
||||
}
|
||||
|
||||
// Stop a device dead, without knowing anything about it.
|
||||
//
|
||||
// Clearing PCI_CMD_BUS_MASTER is the one halt available to a kernel that does
|
||||
// not know what the device is: bus mastering lives in the standard config
|
||||
// header of every PCI device, and with it clear the device cannot start another
|
||||
// DMA cycle. Everything else -- quiescing a queue, parking a scanout, waiting
|
||||
// for in-flight work -- is device-specific, which is exactly the knowledge that
|
||||
// was moved out of the kernel on purpose.
|
||||
//
|
||||
// **This has to happen before the DMA buffers go back to the arena.** A driver
|
||||
// that died left its device configured and running: virtio-gpu with a scanout
|
||||
// set keeps reading its framebuffer for ever, and that framebuffer is about to
|
||||
// be handed to whoever asks next. The failure is not a crash, which is why it
|
||||
// is worth writing down -- it is a display showing another program's memory, or
|
||||
// a NIC writing packets into a buffer a different driver is using.
|
||||
static void
|
||||
pciuser_stop(int idx)
|
||||
{
|
||||
const struct pci_dev *d = pci_get(idx);
|
||||
if (d)
|
||||
pci_disable((struct pci_dev *)d, PCI_CMD_BUS_MASTER);
|
||||
}
|
||||
|
||||
// Release everything a process held. Called when it exits, because a driver
|
||||
// that crashed must not keep its device locked away from its replacement --
|
||||
// that is the entire point of running drivers in processes.
|
||||
void
|
||||
pciuser_release_all(struct proc *p)
|
||||
{
|
||||
acquire(&pciuser_lock);
|
||||
for (int i = 0; i < NPCI; i++)
|
||||
if (owner[i] == p)
|
||||
// Devices first, and outside the lock, because stopping one is a config-space
|
||||
// write and the ordering against the DMA free below is the whole point.
|
||||
for (int i = 0; i < NPCI; i++) {
|
||||
int mine;
|
||||
acquire(&pciuser_lock);
|
||||
mine = (owner[i] == p);
|
||||
if (mine)
|
||||
owner[i] = 0;
|
||||
release(&pciuser_lock);
|
||||
if (mine)
|
||||
pciuser_stop(i);
|
||||
}
|
||||
|
||||
acquire(&pciuser_lock);
|
||||
|
||||
// And its interrupt line -- unregistered, and left MASKED.
|
||||
//
|
||||
|
|
@ -277,7 +310,12 @@ do_claim(uint64 argp)
|
|||
// Bus mastering and memory space, because a driver that has to ask for this
|
||||
// separately will forget, and the symptom -- DMA that silently does nothing
|
||||
// -- costs a day to find.
|
||||
pci_enable((struct pci_dev *)d, 0x0006);
|
||||
//
|
||||
// This is also what makes a device usable again after the driver holding it
|
||||
// died: release clears bus mastering to stop the device, so claim has to put
|
||||
// it back. The two are a pair, and the second driver to run is the one that
|
||||
// finds out if they are not.
|
||||
pci_enable((struct pci_dev *)d, PCI_CMD_MEMORY | PCI_CMD_BUS_MASTER);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -291,9 +329,17 @@ do_release(uint64 argp)
|
|||
return -ENODEV;
|
||||
|
||||
acquire(&pciuser_lock);
|
||||
if (owner[idx] == myproc())
|
||||
int mine = (owner[idx] == myproc());
|
||||
if (mine)
|
||||
owner[idx] = 0;
|
||||
release(&pciuser_lock);
|
||||
|
||||
// Same halt as the death path, for the same reason. A driver that releases
|
||||
// politely has usually quiesced its device already, but "usually" is not a
|
||||
// property the kernel can check, and the cost of being sure is one config
|
||||
// write.
|
||||
if (mine)
|
||||
pciuser_stop(idx);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -314,33 +360,63 @@ do_dma(uint64 argp)
|
|||
if (size == 0 || size > 4 * 1024 * 1024)
|
||||
return -EINVAL;
|
||||
|
||||
// A slot to remember it by, found before allocating: handing out memory that
|
||||
// cannot be recorded is how the leak this fixes got written in the first
|
||||
// place.
|
||||
struct proc *p = myproc();
|
||||
|
||||
// Two slots, both found before anything is allocated: one to remember the
|
||||
// block by, one to remember the mapping by. Handing out memory that cannot
|
||||
// be recorded is how the leak this fixes got written in the first place.
|
||||
//
|
||||
// The VMA is not bookkeeping. It is the thing that keeps this memory out of
|
||||
// kfree()'s hands. freevm() frees every mapped page below PHYSTOP, and the
|
||||
// DMA arena is the top 16 MB of exactly that range -- so a DMA block left
|
||||
// for freevm() to find goes onto the page allocator's free list when the
|
||||
// process dies, while pciuser_release_all() hands the same pages back to
|
||||
// the arena. Both allocators then believe they own it.
|
||||
//
|
||||
// That is what broke H2. The restarted vgpud asked for a 4 MB framebuffer,
|
||||
// dma_alloc() returned pages kalloc() had already given to somebody --
|
||||
// including the page tables of the process doing the asking -- and zeroed
|
||||
// them, so the driver died inside this ioctl, on the return from a syscall
|
||||
// whose stack had just stopped existing.
|
||||
//
|
||||
// no_free says these pages are not freevm()'s to release;
|
||||
// proc_release_shared() unmaps the range before the page tables go away.
|
||||
struct dmablk *blk = 0;
|
||||
acquire(&pciuser_lock);
|
||||
for (int i = 0; i < PCIUSER_MAXDMA; i++)
|
||||
if (!dmablk[i].proc) { blk = &dmablk[i]; blk->proc = myproc(); break; }
|
||||
if (!dmablk[i].proc) { blk = &dmablk[i]; blk->proc = p; break; }
|
||||
release(&pciuser_lock);
|
||||
if (!blk)
|
||||
return -ENOMEM;
|
||||
|
||||
void *kva = dma_alloc(size, PGSIZE);
|
||||
if (!kva) {
|
||||
struct vma *v = 0;
|
||||
for (int i = 0; i < NVMA; i++)
|
||||
if (!p->vma[i].used) { v = &p->vma[i]; break; }
|
||||
if (!v) {
|
||||
blk->proc = 0;
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
// Its own mapping in the caller, taken from the same bump allocator mmap
|
||||
// uses so the two cannot collide.
|
||||
struct proc *p = myproc();
|
||||
if (p->mmap_next == 0)
|
||||
p->mmap_next = 0x40000000;
|
||||
uint64 va = p->mmap_next;
|
||||
if (va + size < va || va + size > KERNBASE) {
|
||||
blk->proc = 0;
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
void *kva = dma_alloc(size, PGSIZE);
|
||||
if (!kva) {
|
||||
blk->proc = 0;
|
||||
return -ENOMEM;
|
||||
}
|
||||
p->mmap_next += size;
|
||||
|
||||
uint64 phys = dma_phys(kva);
|
||||
if (mappages(p->pgdir, (void *)va, size, phys, PTE_W | PTE_U) < 0) {
|
||||
vm_unmap_norelease(p->pgdir, va, size); // whatever part of it took
|
||||
dma_free(kva, size);
|
||||
blk->proc = 0;
|
||||
return -ENOMEM;
|
||||
|
|
@ -348,6 +424,12 @@ do_dma(uint64 argp)
|
|||
blk->kva = kva;
|
||||
blk->size = size;
|
||||
|
||||
v->addr = va;
|
||||
v->length = size;
|
||||
v->used = 1;
|
||||
v->no_free = 1;
|
||||
v->shmid = -1;
|
||||
|
||||
req->uaddr = va;
|
||||
req->paddr = phys;
|
||||
return 0;
|
||||
|
|
@ -422,11 +504,25 @@ do_cfg(uint64 argp)
|
|||
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;
|
||||
|
||||
// The standard header -- the first 64 bytes -- is readable without holding
|
||||
// the device. Refusing it was inconsistent rather than careful: PCIIOC_INFO
|
||||
// already hands back the vendor, device, class, interrupt line and every BAR,
|
||||
// all of which are read out of exactly these bytes. Denying the same bytes in
|
||||
// a different shape protects nothing and makes the bus unenumerable by
|
||||
// anything that is not already driving something.
|
||||
//
|
||||
// Past 0x40 is the capability list and whatever the device puts after it.
|
||||
// That is device-specific, it is what a driver reads to find its own
|
||||
// registers, and it belongs to whoever holds the device.
|
||||
//
|
||||
// Reads only. There is no config *write* here at all, and that is the line
|
||||
// that matters -- a write is how you move a BAR out from under the kernel.
|
||||
if (c->offset >= 0x40 && owner[idx] != myproc())
|
||||
return -EACCES;
|
||||
|
||||
const struct pci_dev *d = pci_get((int)idx);
|
||||
if (!d)
|
||||
return -ENODEV;
|
||||
|
|
|
|||
|
|
@ -225,13 +225,19 @@ const struct display_ops vga_display_ops = {
|
|||
// 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
|
||||
int
|
||||
vga_fb_restore(void)
|
||||
{
|
||||
struct fb_info info;
|
||||
|
||||
// Zero means there is no framebuffer to go back to, and the caller has to
|
||||
// care. It used to return void and say nothing here, which reads as "done"
|
||||
// -- so on a machine whose only display is the one being handed over, the
|
||||
// eviction went ahead and left the console drawing into a driver that had
|
||||
// just freed its memory. The answer is not to try harder; it is to say no,
|
||||
// which is what the caller now does.
|
||||
if (!fb || !fb_width || !fb_height)
|
||||
return; // nothing was ever set up; nothing to go back to
|
||||
return 0;
|
||||
|
||||
info.ptr = fb;
|
||||
info.phys = fb_phys;
|
||||
|
|
@ -241,4 +247,5 @@ vga_fb_restore(void)
|
|||
info.width = fb_width;
|
||||
info.height = fb_height;
|
||||
gfx_attach_scanout(&info);
|
||||
return 1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -537,7 +537,21 @@ vgpu_release(struct pci_dev *d)
|
|||
if (!g)
|
||||
return -1;
|
||||
|
||||
vga_fb_restore();
|
||||
// The rule from docs/plan/hybrid.md, enforced rather than described: **the
|
||||
// kernel keeps a minimal driver for everything on the path to a shell.** If
|
||||
// there is no framebuffer to fall back to, this device is the only screen the
|
||||
// machine has, and handing it to a process that might crash would make a
|
||||
// failed driver indistinguishable from a dead machine.
|
||||
//
|
||||
// Refusing here is what lets the userspace driver be started unconditionally
|
||||
// at boot: on hardware where it is safe it takes the GPU, and on hardware
|
||||
// where it is not it is told no and says so, rather than each machine needing
|
||||
// to be configured for its own display topology.
|
||||
if (!vga_fb_restore()) {
|
||||
cprintf("virtio-gpu: refusing to release -- it is the only display, and "
|
||||
"there is no framebuffer to fall back to\n");
|
||||
return -1;
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue