usb/fat: transfer events matched by slot+endpoint; storage failures heal
B2 — the 1-in-3 boot-time READ CAPACITY failure, root-caused: the xHCI library's awaitTransfer claimed ANY unclaimed transfer event as its own completion. An interrupt-endpoint event whose TRB pointer no longer matched the armed subscription (an error or stale completion from the keyboard/mouse polling concurrently with storage bring-up) fell through and was misread as the bulk transfer's completion — desynchronizing the mass-storage bulk protocol in controller state that SURVIVED driver restarts, so every retry failed too. Awaited transfers now match the event's slot id and endpoint DCI; foreign events are dropped and named. Twelve consecutive runs of the previously-flaky cases pass; the full suite is green with none of its old intermittents. B1 — and when storage does fail transiently, the system now heals instead of giving up forever: a nonzero exit maps to ExitReason.aborted (a deliberate FAILURE exit — supervisors restart those with backoff, unlike a clean .exited), usb-storage exits nonzero when a PRESENT device fails bring-up, and the fat service no longer blocks its harness polling for a block device and then dies — it serves immediately (requests fail politely), retries on a 500 ms timer, and mounts whenever storage appears, including after a driver restart.
This commit is contained in:
@@ -61,6 +61,12 @@ fn transact(cdb: []const u8, direction_in: bool, data_physical: u64, data_length
|
||||
return status.status == @intFromEnum(bot.CommandStatus.passed);
|
||||
}
|
||||
|
||||
/// Set when bring-up failed with the device PRESENT (an opened device that then
|
||||
/// failed a step): main exits nonzero, and the device manager restarts us with
|
||||
/// backoff — a transient failure heals instead of leaving storage down forever.
|
||||
/// Device-absent paths stay clean exits: nothing to serve, nothing to retry.
|
||||
var bring_up_failed = false;
|
||||
|
||||
fn initialise(endpoint: runtime.ipc.Handle) bool {
|
||||
_ = endpoint;
|
||||
if (!runtime.usb.helloManager(device_id)) {
|
||||
@@ -73,10 +79,12 @@ fn initialise(endpoint: runtime.ipc.Handle) bool {
|
||||
};
|
||||
bulk_in = device.findEndpoint(runtime.usb.transfer_type_bulk, true) orelse {
|
||||
_ = runtime.system.write("/system/drivers/usb-storage: no bulk-IN endpoint\n");
|
||||
bring_up_failed = true;
|
||||
return false;
|
||||
};
|
||||
bulk_out = device.findEndpoint(runtime.usb.transfer_type_bulk, false) orelse {
|
||||
_ = runtime.system.write("/system/drivers/usb-storage: no bulk-OUT endpoint\n");
|
||||
bring_up_failed = true;
|
||||
return false;
|
||||
};
|
||||
command_wrapper = dma.alloc(4096, dma.coherent) orelse return false;
|
||||
@@ -99,6 +107,7 @@ fn initialise(endpoint: runtime.ipc.Handle) bool {
|
||||
const capacity_command = scsi.readCapacity10();
|
||||
if (!transact(&capacity_command, true, command_data.physical, 8)) {
|
||||
_ = runtime.system.write("/system/drivers/usb-storage: READ CAPACITY failed\n");
|
||||
bring_up_failed = true;
|
||||
return false;
|
||||
}
|
||||
var capacity_bytes: [8]u8 = undefined;
|
||||
@@ -174,4 +183,7 @@ pub fn main(init: runtime.process.Init) void {
|
||||
.init = initialise,
|
||||
.on_message = onMessage,
|
||||
});
|
||||
// A failure exit (nonzero -> .aborted) tells the device manager to restart
|
||||
// us with backoff; a clean return means there was nothing to serve.
|
||||
if (bring_up_failed) runtime.system.exit(1);
|
||||
}
|
||||
|
||||
@@ -682,16 +682,29 @@ pub const Controller = struct {
|
||||
return null;
|
||||
}
|
||||
|
||||
fn awaitTransfer(self: *Controller, requested_length: u32) ?u8 {
|
||||
/// Await the completion of OUR transfer — identified by the event's slot id
|
||||
/// (control[31:24]) and endpoint DCI (control[20:16]). Any other transfer
|
||||
/// event is either a subscription's report (serviced) or foreign noise (an
|
||||
/// interrupt endpoint's error/stale completion whose TRB pointer no longer
|
||||
/// matches the armed one) — DROPPED, never misattributed: claiming a foreign
|
||||
/// event as our completion desynchronized the mass-storage bulk protocol in
|
||||
/// a way that survived every driver restart (the 1-in-3 READ CAPACITY
|
||||
/// failure at boot, with a USB keyboard and mouse polling concurrently).
|
||||
fn awaitTransfer(self: *Controller, slot_id: u8, dci: u32, requested_length: u32) ?u8 {
|
||||
const deadline = system.clock() + 1_000_000_000;
|
||||
while (true) {
|
||||
const event = self.nextEvent(deadline) orelse return null;
|
||||
if (trbType(event.control) == @intFromEnum(TrbType.transfer_event)) {
|
||||
if (self.serviceInterruptEvent(event)) continue; // a subscription's report
|
||||
const residual = event.status & 0xFFFFFF;
|
||||
self.last_transfer_length = if (residual >= requested_length) 0 else requested_length - residual;
|
||||
return completionCode(event.status); // our transfer's completion (or error)
|
||||
if (trbType(event.control) != @intFromEnum(TrbType.transfer_event)) continue;
|
||||
if (self.serviceInterruptEvent(event)) continue; // a subscription's report
|
||||
const event_slot: u8 = @truncate(event.control >> 24);
|
||||
const event_dci: u32 = (event.control >> 16) & 0x1F;
|
||||
if (event_slot != slot_id or event_dci != dci) {
|
||||
std.log.info("dropped foreign transfer event (slot {d} dci {d}, code {d})", .{ event_slot, event_dci, completionCode(event.status) });
|
||||
continue;
|
||||
}
|
||||
const residual = event.status & 0xFFFFFF;
|
||||
self.last_transfer_length = if (residual >= requested_length) 0 else requested_length - residual;
|
||||
return completionCode(event.status); // our transfer's completion (or error)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -732,7 +745,7 @@ pub const Controller = struct {
|
||||
|
||||
mmio.wmb();
|
||||
self.ringDoorbell(device.slot_id, 1); // DCI 1 = EP0
|
||||
const code = self.awaitTransfer(@intCast(data.len)) orelse return false;
|
||||
const code = self.awaitTransfer(device.slot_id, 1, @intCast(data.len)) orelse return false;
|
||||
if (code != @intFromEnum(CompletionCode.success) and code != @intFromEnum(CompletionCode.short_packet)) return false;
|
||||
|
||||
if (has_data and direction_in) {
|
||||
@@ -925,8 +938,9 @@ pub const Controller = struct {
|
||||
mmio.wmb();
|
||||
const number: u8 = endpoint.address & 0x0F;
|
||||
const direction_in = endpoint.address & 0x80 != 0;
|
||||
self.ringDoorbell(device.slot_id, doorbellContextIndex(number, direction_in));
|
||||
const code = self.awaitTransfer(length) orelse return null;
|
||||
const dci = doorbellContextIndex(number, direction_in);
|
||||
self.ringDoorbell(device.slot_id, dci);
|
||||
const code = self.awaitTransfer(device.slot_id, dci, length) orelse return null;
|
||||
if (code != @intFromEnum(CompletionCode.success) and code != @intFromEnum(CompletionCode.short_packet)) return null;
|
||||
return self.last_transfer_length;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user