From a44b397bed8c5f00c1185490789cc757396ee89d Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:18:21 +0100 Subject: [PATCH] =?UTF-8?q?protocols:=20attach=20gets=20its=20reverse=20?= =?UTF-8?q?=E2=80=94=20block=20detach,=20usb-transfer=20dma=5Fdetach?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kernel was always symmetric (dma_bind 51 / dma_unbind 52); the two protocols that forward an attachment up the stack were one-way, so a live client could grant a device reach into its buffer but never revoke it while alive — exactly the one-way lifecycle the storage architecture's enforcement section forbids. Death stays the mechanical backstop; detach is the living process's path. Both verbs are appended, so every existing number holds. The shape mirrors attach precisely: the same region capability rides the cap slot again — the kernel matches the region, so no layer retains anything between the calls (the bus never kept the handle; now it never needs to). fat's bring-up does attach -> detach -> attach, exercising both verbs through the whole chain (fat -> storage -> bus -> kernel) on every boot: a broken detach fails every fat case instead of lying dormant until the first buffer replacement. Honest scope: the round trip proves the plumbing; unbind semantics are the kernel iommu tests' (map/unmap/translationOf); the full composition (detach then DMA faults) is a future iommu-fault extension. --- library/device/block/block.zig | 8 ++++++++ library/device/usb/usb.zig | 9 +++++++++ library/protocol/block/block-protocol.zig | 6 ++++++ .../protocol/usb-transfer/usb-transfer-protocol.zig | 6 ++++++ system/drivers/usb-storage/usb-storage.zig | 8 ++++++++ system/drivers/usb-xhci-bus/usb-xhci-bus.zig | 9 +++++++++ system/services/fat/fat.zig | 12 ++++++++++++ 7 files changed, 58 insertions(+) diff --git a/library/device/block/block.zig b/library/device/block/block.zig index b03e4e9..cba4c16 100644 --- a/library/device/block/block.zig +++ b/library/device/block/block.zig @@ -35,6 +35,14 @@ pub const Device = struct { return self.call(.attach, {}, handle, &reply) != null; } + /// The reverse of `attach`: the buffer leaves the device's reach. The same + /// region capability rides again (the kernel matches the region). Do not name + /// the buffer's physical address in `read`/`write` after this. + pub fn detach(self: Device, handle: ipc.Handle) bool { + var reply: [block_protocol.message_maximum]u8 = undefined; + return self.call(.detach, {}, handle, &reply) != null; + } + /// Read `count` blocks starting at `lba` into the DMA buffer at `physical`. pub fn read(self: Device, lba: u64, count: u32, physical: u64) bool { var reply: [block_protocol.message_maximum]u8 = undefined; diff --git a/library/device/usb/usb.zig b/library/device/usb/usb.zig index b620d07..997f3fa 100644 --- a/library/device/usb/usb.zig +++ b/library/device/usb/usb.zig @@ -136,6 +136,15 @@ pub const Device = struct { return self.call(.dma_attach, {}, &.{}, handle, &reply) != null; } + /// The reverse of `attachDma`: unbind the buffer from the controller's IOMMU + /// domain. The same region capability rides again — the kernel matches the + /// region, so neither side kept state between the two calls. Do not name the + /// buffer's physical address in any transfer after this. + pub fn detachDma(self: *Device, handle: ipc.Handle) bool { + var reply: [usb_transfer_protocol.message_maximum]u8 = undefined; + return self.call(.dma_detach, {}, &.{}, handle, &reply) != null; + } + /// One bulk transfer (IN or OUT per `endpoint_address`'s direction bit) to or /// from the caller's own DMA buffer at `physical`. Returns the bytes moved. pub fn bulk(self: *Device, endpoint_address: u8, physical: u64, length: u32) ?u32 { diff --git a/library/protocol/block/block-protocol.zig b/library/protocol/block/block-protocol.zig index 339c7fe..cc960ec 100644 --- a/library/protocol/block/block-protocol.zig +++ b/library/protocol/block/block-protocol.zig @@ -54,6 +54,12 @@ pub const Protocol = envelope.Define(.{ // physical addresses (named in later read/write) are reachable by the // device under an enforcing IOMMU. Call once per buffer before using it. .{ .name = "attach" }, + // detach(): the reverse — the same region capability rides the cap slot + // (the caller still holds its handle; the kernel matches the region) and + // the buffer leaves the device's domain. Every grant a live process + // makes is revocable by the granter while alive; death remains the + // mechanical backstop (storage-architecture.md, the lifecycle rule). + .{ .name = "detach" }, }, }); diff --git a/library/protocol/usb-transfer/usb-transfer-protocol.zig b/library/protocol/usb-transfer/usb-transfer-protocol.zig index f23d404..0a95611 100644 --- a/library/protocol/usb-transfer/usb-transfer-protocol.zig +++ b/library/protocol/usb-transfer/usb-transfer-protocol.zig @@ -156,6 +156,11 @@ pub const Protocol = envelope.Define(.{ // target says which caller's device is attaching, so there is nothing // left for a body to carry. .{ .name = "dma_attach" }, + // dma_detach: the reverse, same shape — the region capability rides the + // cap slot again (the kernel matches the region; the provider retains + // nothing between the two calls) and the buffer leaves the controller's + // domain. Appended, so every existing verb keeps its number. + .{ .name = "dma_detach" }, }, .events = &.{ .{ .name = "interrupt_report", .payload = InterruptReport }, @@ -188,6 +193,7 @@ test "the verb numbering, and the device token in the header" { try std.testing.expectEqual(@as(u32, 18), @intFromEnum(Operation.interrupt_subscribe)); try std.testing.expectEqual(@as(u32, 19), @intFromEnum(Operation.bulk)); try std.testing.expectEqual(@as(u32, 20), @intFromEnum(Operation.dma_attach)); + try std.testing.expectEqual(@as(u32, 21), @intFromEnum(Operation.dma_detach)); try std.testing.expectEqual(@as(u32, 16), @intFromEnum(Event.interrupt_report)); var buffer: [message_maximum]u8 = undefined; diff --git a/system/drivers/usb-storage/usb-storage.zig b/system/drivers/usb-storage/usb-storage.zig index efa49b3..3aebf5a 100644 --- a/system/drivers/usb-storage/usb-storage.zig +++ b/system/drivers/usb-storage/usb-storage.zig @@ -204,12 +204,20 @@ fn onAttach(_: void, invocation: Invocation(void), _: Answer(void)) isize { return if (device.attachDma(handle)) 0 else refused; } +/// The reverse: forward the same region capability so the controller unbinds +/// the buffer. As with attach, our copy stays the turn's to close. +fn onDetach(_: void, invocation: Invocation(void), _: Answer(void)) isize { + const handle = invocation.capability orelse return -envelope.EPROTO; + return if (device.detachDma(handle)) 0 else refused; +} + const handlers = Serve.Handlers{ .geometry = onGeometry, .read = onRead, .write = onWrite, .flush = onFlush, .attach = onAttach, + .detach = onDetach, }; fn onMessage(message: []const u8, reply: []u8, sender: u32, arrived: *ipc.Arrival) usize { diff --git a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig index b85f586..bcc6100 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig @@ -593,6 +593,7 @@ const handlers = Serve.Handlers{ .interrupt_subscribe = onInterruptSubscribe, .bulk = onBulk, .dma_attach = onDmaAttach, + .dma_detach = onDmaDetach, }; /// open: the target is the class driver's assigned device id. Resolve it to an @@ -694,6 +695,14 @@ fn onDmaAttach(_: void, invocation: Invocation(void), _: Answer(void)) isize { return if (device.dmaBind(controller_id, handle)) 0 else refused; } +/// The reverse: the same region capability arrives again and the buffer leaves +/// the controller's domain (`dma_unbind` matches the region — nothing was +/// retained here between the two calls). The turn closes the arriving copy. +fn onDmaDetach(_: void, invocation: Invocation(void), _: Answer(void)) isize { + const handle = invocation.capability orelse return -envelope.EPROTO; + return if (device.dmaUnbind(controller_id, handle)) 0 else refused; +} + /// A timer tick or an MSI landed: drain the event ring, reconcile ports, and fan out. /// The timer arm re-arms itself (8 ms drain when polling, 250 ms reconcile under MSI); /// the MSI arm clears the interrupter's pending bit FIRST, then drains — so an event diff --git a/system/services/fat/fat.zig b/system/services/fat/fat.zig index 6b81fd6..30d4626 100644 --- a/system/services/fat/fat.zig +++ b/system/services/fat/fat.zig @@ -196,10 +196,22 @@ fn tryBringUp() void { // enforcing IOMMU. No-op binding otherwise. const bounce = memory.dmaAlloc(engine.max_transfer_sectors * 512, memory.dma_coherent | memory.dma_shareable) orelse return; if (bounce.handle) |handle| { + // Attach, detach, and attach again: the round trip exercises BOTH verbs + // of the DMA-window lifecycle through the whole chain (fat → storage → + // bus → kernel) on every boot, so a broken detach fails every fat case + // rather than lying dormant until the first buffer replacement. if (!device.attach(handle)) { _ = logging.write("/system/services/fat: could not attach the DMA bounce buffer\n"); return; } + if (!device.detach(handle)) { + _ = logging.write("/system/services/fat: could not detach the DMA bounce buffer\n"); + return; + } + if (!device.attach(handle)) { + _ = logging.write("/system/services/fat: could not re-attach the DMA bounce buffer\n"); + return; + } _ = ipc.close(handle); // the binding holds its own reference now } ipc_block = .{ .device = device, .bounce = bounce };