From 8bf6965340d04d4883cbdee015f5670a5f0c0a7a Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 22:18:50 +0200 Subject: [PATCH 1/7] netstack: Card moves to card.rs and the client's types to client.rs Moves only. `Card` and its `impl` go to `card.rs`; `Client`, `Request`, `PendingConn`, `ClientRx`, the two response words and the three constants that bound a pending client go to `client.rs`. The four poller-sizing constants stay in `main.rs`: they size the loop's poller, and one of them names `resolve::MAX_LOOKUPS`. What `git show --color-moved` shows beside the moved lines: the two `mod` lines, the `use` lines each file now needs, `pub` on the items, fields and methods `main.rs` names across the module boundary, and one section banner (`// --- One request, and the client waiting for its answer ---`) that is deleted because the file's name now says it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- userland/netstack/src/card.rs | 143 ++++++++++++++++ userland/netstack/src/client.rs | 139 ++++++++++++++++ userland/netstack/src/main.rs | 280 +------------------------------- 3 files changed, 288 insertions(+), 274 deletions(-) create mode 100644 userland/netstack/src/card.rs create mode 100644 userland/netstack/src/client.rs diff --git a/userland/netstack/src/card.rs b/userland/netstack/src/card.rs new file mode 100644 index 00000000000..55ad26666a4 --- /dev/null +++ b/userland/netstack/src/card.rs @@ -0,0 +1,143 @@ +use toyos_i219::Part; +use toyos_inspect::Snapshot; + +use crate::i219; +use crate::virtio_net::VirtioNet; + +/// The NIC this program drives, whichever one the manifest gave it. +/// +/// **One enum and not a trait object**: there are two of them, both known at +/// build time, and what a `dyn` would buy is a vtable on the frame path. +pub enum Card { + Virtio(VirtioNet), + Intel(i219::Nic), +} + +impl Card { + /// A device this driver cannot bring up is not a machine without a NIC: the + /// claim was minted, so something the device said is not what this driver + /// understands, and that is loud. + fn undrivable(why: impl std::fmt::Display) -> ! { + panic!("netstack: the NIC this program was given is not one it can drive — {why}") + } + + pub fn intel(claim: toyos::PciDev, part: Part) -> Self { + match i219::Nic::open(claim, part) { + Ok(nic) => Self::Intel(nic), + Err(why) => Self::undrivable(why), + } + } + + pub fn virtio(claim: toyos::PciDev) -> Self { + match VirtioNet::open(claim) { + Ok(nic) => Self::Virtio(nic), + Err(why) => Self::undrivable(why), + } + } + + pub fn mac(&self) -> [u8; 6] { + match self { + Self::Virtio(nic) => nic.mac(), + Self::Intel(nic) => nic.mac(), + } + } + + /// The claim, for the poller: readable means an interrupt has landed. + pub fn claim(&self) -> &toyos::PciDev { + match self { + Self::Virtio(nic) => nic.claim(), + Self::Intel(nic) => nic.claim(), + } + } + + /// **The record has to be taken, not merely noticed.** A claim reads ready + /// while it holds an undrained interrupt, so a pass that saw the token and + /// left it would find the same one on the next `wait` and every one after + /// it. What the message meant is in the rings, which `iface.poll` reads. + /// This is also where a driver with a per-pass budget gets it back. + /// + /// A claim that refuses the read for anything but `WouldBlock` is the + /// kernel saying this function is no longer this process's: a fault at the + /// unit is the one that happens, and by the time it is answered the + /// function's bus mastering is gone. Every frame from here on is one that + /// silently never arrives, so this dies where it can be read — once, for + /// whichever driver is running. + /// + /// Answers the link where the pass found it changed; virtio reports none. + pub fn begin_pass(&self) -> Option { + let answered = match self { + Self::Virtio(nic) => nic.take_interrupt().map(|_| None), + Self::Intel(nic) => nic.begin_pass(), + }; + answered + .unwrap_or_else(|why| panic!("netstack: this NIC's claim refused an interrupt read: {why:?}")) + } + + /// What `inspect` reads about the card: which driver, its address, its + /// link, and on the Intel parts what the driver and the MAC counted. + /// + /// **virtio's link is `unreported`, not `up`**: the device tells netstack + /// nothing about one, and netstack serving as though it were up is netstack's + /// assumption rather than something it measured. + pub fn inspect(&self, snap: &mut Snapshot) { + let m = self.mac(); + snap.put( + "mac", + format!("{:02x}:{:02x}:{:02x}:{:02x}:{:02x}:{:02x}", m[0], m[1], m[2], m[3], m[4], m[5]), + ); + let nic = match self { + Self::Virtio(_) => { + snap.put("driver", "virtio-net"); + snap.put("link.state", "unreported"); + return; + } + Self::Intel(nic) => nic, + }; + snap.put( + "driver", + match nic.part() { + Part::I219 => "i219", + Part::E82574 => "82574", + }, + ); + match nic.link() { + toyos_i219::Link::Down => snap.put("link.state", "down"), + toyos_i219::Link::Up { speed, full_duplex } => { + snap.put("link.state", "up"); + snap.put( + "link.speed_mbps", + match speed { + toyos_i219::Speed::Mbps10 => 10u32, + toyos_i219::Speed::Mbps100 => 100, + toyos_i219::Speed::Mbps1000 => 1000, + }, + ); + snap.put("link.duplex", if full_duplex { "full" } else { "half" }); + } + } + let (counters, wire) = nic.counts(); + snap.put("descriptors.sent", counters.sent); + snap.put("descriptors.received", counters.received); + snap.put("wire.sent", wire.sent); + snap.put("wire.received", wire.received); + snap.put("wire.seen", wire.seen); + snap.put("errors.missed", wire.missed); + snap.put("errors.crc", wire.crc_errors); + } + + pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { + match self { + Self::Virtio(nic) => nic.tx(len, fill), + Self::Intel(nic) => nic.tx(len, fill), + } + } + + /// Say what the driver counted, once a pass and after every frame the pass + /// sent: a line per dropped frame is itself more frames to send. + pub fn report(&self) { + match self { + Self::Virtio(nic) => nic.report(), + Self::Intel(nic) => nic.report(), + } + } +} diff --git a/userland/netstack/src/client.rs b/userland/netstack/src/client.rs new file mode 100644 index 00000000000..d421035c3a9 --- /dev/null +++ b/userland/netstack/src/client.rs @@ -0,0 +1,139 @@ +use std::time::{Duration, Instant}; + +use toyos::AsHandle; +use toyos::ipc::{self, Connection, IpcPayload}; +use toyos::net::{ErrorResponse, RespType}; +use toyos::say; + +const RESP_RESULT: u32 = RespType::Result as u32; +const RESP_ERROR: u32 = RespType::Error as u32; + +/// One client's connection, which is also how netstack names it. +/// +/// netstack answers a connection exactly once and then lets it close, so a handler +/// owns this for as long as its operation lasts: the synchronous ones drop it +/// where they answer, and the three asynchronous ones keep it across passes +/// until what they started finishes. The handle closes with it — which is what +/// replaced a `mem::forget` on the accepted connection and eight hand-written +/// `close` calls that had to agree with each other on every path. +pub struct Client { + pub conn: Connection, +} + +impl Client { + pub fn result(&self, payload: &T) { + self.answered(self.conn.try_send(RESP_RESULT, payload)); + } + + pub fn result_bytes(&self, data: &[u8]) { + self.answered(self.conn.try_send_bytes(RESP_RESULT, data)); + } + + pub fn done(&self) { + self.answered(self.conn.try_signal(RESP_RESULT)); + } + + pub fn error(&self, code: u32) { + self.answered(self.conn.try_send(RESP_ERROR, &ErrorResponse { code })); + } + + pub fn snapshot(&self, encoded: &[u8]) { + self.answered(self.conn.try_send_bytes(toyos_inspect::MSG_SNAPSHOT, encoded)); + } + + /// Whether this client, waiting for its answer, has left. A connection + /// carries one request, so hanging up is the one thing it may say while + /// it waits; a client that says more is dropped by name. + pub fn gone(&self) -> bool { + let mut byte = [0u8; 1]; + match self.conn.read_nonblock(&mut byte) { + Err(toyos_abi::syscall::SyscallError::WouldBlock) => false, + Ok(0) | Err(_) => true, + Ok(_) => { + say!("netstack: dropping client {} — it spoke again before its answer", self.conn.as_handle().0); + true + } + } + } + + /// **The answer goes out in one non-blocking write, and a refusal is not + /// retried.** `ipc::send` parks in `sys_write` until the client drains, + /// which is a client deciding when the network stack runs again; and + /// `TrySendError::Full` can have left part of the frame in the pipe, so + /// there is nothing here to retry either. The connection closes either way. + /// The log is the only place the machine this runs on gets told that a + /// client asked something and was never answered. + fn answered(&self, sent: Result<(), ipc::TrySendError>) { + if let Err(e) = sent { + let why = match e { + ipc::TrySendError::Full => { + "its pipe will not take the answer and it is not reading" + } + ipc::TrySendError::TooLarge => "the answer netstack built is larger than a frame", + ipc::TrySendError::Syscall(_) => "its connection is gone", + }; + say!("netstack: dropping client {} — {why}", self.conn.as_handle().0); + } + } +} + +/// Connections accepted and not yet carrying a whole request. +/// +/// The kernel queues 32 unaccepted connections per listener +/// (`listener::MAX_PENDING_CONNECTIONS`); this is the same allowance one step +/// further along, for a client that has been accepted and has not yet said what +/// it wants. Past it netstack refuses by name rather than growing, and +/// [`HANDSHAKE_TIMEOUT`] is what guarantees the table drains. +pub const MAX_PENDING_CONNS: u32 = 32; + +/// How long an accepted connection may go without completing its request. +/// +/// Policy, and generous: every client in the tree sends its request in the +/// statement after `connect` (`toyos::net`'s `NetstackConn::request`). What this +/// bounds is the one that never sends it. +pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(2); + +/// The largest request payload netstack keeps. +/// +/// `MsgType::DnsLookup` is the only request carrying bytes rather than a struct, +/// and `toyos::net::dns_lookup` frames a hostname into a 256-byte buffer; every +/// typed request is far smaller, `TcpConnectPipedRequest` at 32 bytes being the +/// widest. A client may declare anything up to `ipc::MAX_FRAME_LEN` — the excess +/// is counted down and discarded, never waited for. +pub const MAX_KEPT_REQUEST: usize = 256; + +/// One client's inbound framing. +/// +/// **netstack never reads a client with a blocking read.** That is the whole point +/// of [`ipc::FrameRx`]: `ipc::recv_header` and `ipc::recv_payload` park the +/// caller until the peer sends the bytes it promised. Here a peer that stops halfway +/// through a frame costs a buffer and a deadline instead of the event loop. +pub type ClientRx = ipc::FrameRx; + +/// A connection that has been accepted and has not yet said what it wants. +/// +/// It exists because `accept` and the request frame are two events. +pub struct PendingConn { + pub conn: Connection, + pub rx: ClientRx, + pub since: Instant, +} + +/// A whole request, off the connection and in memory. +/// +/// The payload travels with the frame instead of being read off the connection +/// during +/// dispatch: the read side is finished before anything acts on a message, so no +/// handler below can park on the client that sent it. +pub struct Request { + pub client: Client, + pub msg_type: u32, + pub payload: [u8; MAX_KEPT_REQUEST], + pub payload_len: usize, +} + +impl Request { + pub fn payload(&self) -> &[u8] { + &self.payload[..self.payload_len] + } +} diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index 0bd5fcc9fc9..2a7b25a436d 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -3,9 +3,11 @@ use std::time::{Duration, Instant}; use toyos::poller::{OTHER_END_GONE, READABLE, WRITABLE, Poller}; use toyos::ipc; use toyos::AsHandle; -use toyos::ipc::{Connection, IpcPayload, RxStep}; +use toyos::ipc::RxStep; use toyos::say; +mod card; +mod client; mod device; mod dhcp; mod i219; @@ -39,6 +41,9 @@ use toyos_i219::Part; use toyos_inspect::Snapshot; use virtio_net::VirtioNet; +use card::Card; +use client::{Client, ClientRx, HANDSHAKE_TIMEOUT, MAX_KEPT_REQUEST, MAX_PENDING_CONNS, PendingConn, Request}; + use toyos::net::*; use smoltcp::iface::{Config, Interface, PollResult, SocketHandle, SocketSet}; @@ -51,144 +56,6 @@ use std::net::Ipv4Addr; // --- smoltcp Device wrapper --- -/// The NIC this program drives, whichever one the manifest gave it. -/// -/// **One enum and not a trait object**: there are two of them, both known at -/// build time, and what a `dyn` would buy is a vtable on the frame path. -enum Card { - Virtio(VirtioNet), - Intel(i219::Nic), -} - -impl Card { - /// A device this driver cannot bring up is not a machine without a NIC: the - /// claim was minted, so something the device said is not what this driver - /// understands, and that is loud. - fn undrivable(why: impl std::fmt::Display) -> ! { - panic!("netstack: the NIC this program was given is not one it can drive — {why}") - } - - fn intel(claim: toyos::PciDev, part: Part) -> Self { - match i219::Nic::open(claim, part) { - Ok(nic) => Self::Intel(nic), - Err(why) => Self::undrivable(why), - } - } - - fn virtio(claim: toyos::PciDev) -> Self { - match VirtioNet::open(claim) { - Ok(nic) => Self::Virtio(nic), - Err(why) => Self::undrivable(why), - } - } - - fn mac(&self) -> [u8; 6] { - match self { - Self::Virtio(nic) => nic.mac(), - Self::Intel(nic) => nic.mac(), - } - } - - /// The claim, for the poller: readable means an interrupt has landed. - fn claim(&self) -> &toyos::PciDev { - match self { - Self::Virtio(nic) => nic.claim(), - Self::Intel(nic) => nic.claim(), - } - } - - /// **The record has to be taken, not merely noticed.** A claim reads ready - /// while it holds an undrained interrupt, so a pass that saw the token and - /// left it would find the same one on the next `wait` and every one after - /// it. What the message meant is in the rings, which `iface.poll` reads. - /// This is also where a driver with a per-pass budget gets it back. - /// - /// A claim that refuses the read for anything but `WouldBlock` is the - /// kernel saying this function is no longer this process's: a fault at the - /// unit is the one that happens, and by the time it is answered the - /// function's bus mastering is gone. Every frame from here on is one that - /// silently never arrives, so this dies where it can be read — once, for - /// whichever driver is running. - /// - /// Answers the link where the pass found it changed; virtio reports none. - fn begin_pass(&self) -> Option { - let answered = match self { - Self::Virtio(nic) => nic.take_interrupt().map(|_| None), - Self::Intel(nic) => nic.begin_pass(), - }; - answered - .unwrap_or_else(|why| panic!("netstack: this NIC's claim refused an interrupt read: {why:?}")) - } - - /// What `inspect` reads about the card: which driver, its address, its - /// link, and on the Intel parts what the driver and the MAC counted. - /// - /// **virtio's link is `unreported`, not `up`**: the device tells netstack - /// nothing about one, and netstack serving as though it were up is netstack's - /// assumption rather than something it measured. - fn inspect(&self, snap: &mut Snapshot) { - let m = self.mac(); - snap.put( - "mac", - format!("{:02x}:{:02x}:{:02x}:{:02x}:{:02x}:{:02x}", m[0], m[1], m[2], m[3], m[4], m[5]), - ); - let nic = match self { - Self::Virtio(_) => { - snap.put("driver", "virtio-net"); - snap.put("link.state", "unreported"); - return; - } - Self::Intel(nic) => nic, - }; - snap.put( - "driver", - match nic.part() { - Part::I219 => "i219", - Part::E82574 => "82574", - }, - ); - match nic.link() { - toyos_i219::Link::Down => snap.put("link.state", "down"), - toyos_i219::Link::Up { speed, full_duplex } => { - snap.put("link.state", "up"); - snap.put( - "link.speed_mbps", - match speed { - toyos_i219::Speed::Mbps10 => 10u32, - toyos_i219::Speed::Mbps100 => 100, - toyos_i219::Speed::Mbps1000 => 1000, - }, - ); - snap.put("link.duplex", if full_duplex { "full" } else { "half" }); - } - } - let (counters, wire) = nic.counts(); - snap.put("descriptors.sent", counters.sent); - snap.put("descriptors.received", counters.received); - snap.put("wire.sent", wire.sent); - snap.put("wire.received", wire.received); - snap.put("wire.seen", wire.seen); - snap.put("errors.missed", wire.missed); - snap.put("errors.crc", wire.crc_errors); - } - - fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { - match self { - Self::Virtio(nic) => nic.tx(len, fill), - Self::Intel(nic) => nic.tx(len, fill), - } - } - - /// Say what the driver counted, once a pass and after every frame the pass - /// sent: a line per dropped frame is itself more frames to send. - fn report(&self) { - match self { - Self::Virtio(nic) => nic.report(), - Self::Intel(nic) => nic.report(), - } - } -} - /// The driver, as smoltcp's `Device`. /// /// A thin adapter: every token below borrows the driver rather than a claim @@ -489,109 +356,10 @@ fn piped_connection(socket_id: u32, handle: SocketHandle, pipes: DataPipes) -> P } } -// --- One request, and the client waiting for its answer --- - -const RESP_RESULT: u32 = RespType::Result as u32; -const RESP_ERROR: u32 = RespType::Error as u32; - -/// One client's connection, which is also how netstack names it. -/// -/// netstack answers a connection exactly once and then lets it close, so a handler -/// owns this for as long as its operation lasts: the synchronous ones drop it -/// where they answer, and the three asynchronous ones keep it across passes -/// until what they started finishes. The handle closes with it — which is what -/// replaced a `mem::forget` on the accepted connection and eight hand-written -/// `close` calls that had to agree with each other on every path. -struct Client { - conn: Connection, -} - -impl Client { - fn result(&self, payload: &T) { - self.answered(self.conn.try_send(RESP_RESULT, payload)); - } - - fn result_bytes(&self, data: &[u8]) { - self.answered(self.conn.try_send_bytes(RESP_RESULT, data)); - } - - fn done(&self) { - self.answered(self.conn.try_signal(RESP_RESULT)); - } - - fn error(&self, code: u32) { - self.answered(self.conn.try_send(RESP_ERROR, &ErrorResponse { code })); - } - - fn snapshot(&self, encoded: &[u8]) { - self.answered(self.conn.try_send_bytes(toyos_inspect::MSG_SNAPSHOT, encoded)); - } - - /// Whether this client, waiting for its answer, has left. A connection - /// carries one request, so hanging up is the one thing it may say while - /// it waits; a client that says more is dropped by name. - fn gone(&self) -> bool { - let mut byte = [0u8; 1]; - match self.conn.read_nonblock(&mut byte) { - Err(toyos_abi::syscall::SyscallError::WouldBlock) => false, - Ok(0) | Err(_) => true, - Ok(_) => { - say!("netstack: dropping client {} — it spoke again before its answer", self.conn.as_handle().0); - true - } - } - } - - /// **The answer goes out in one non-blocking write, and a refusal is not - /// retried.** `ipc::send` parks in `sys_write` until the client drains, - /// which is a client deciding when the network stack runs again; and - /// `TrySendError::Full` can have left part of the frame in the pipe, so - /// there is nothing here to retry either. The connection closes either way. - /// The log is the only place the machine this runs on gets told that a - /// client asked something and was never answered. - fn answered(&self, sent: Result<(), ipc::TrySendError>) { - if let Err(e) = sent { - let why = match e { - ipc::TrySendError::Full => { - "its pipe will not take the answer and it is not reading" - } - ipc::TrySendError::TooLarge => "the answer netstack built is larger than a frame", - ipc::TrySendError::Syscall(_) => "its connection is gone", - }; - say!("netstack: dropping client {} — {why}", self.conn.as_handle().0); - } - } -} - /// Poll registrations that are not piped connections: the service listener and /// the NIC claim. const FIXED_POLL_HANDLES: u32 = 2; -/// Connections accepted and not yet carrying a whole request. -/// -/// The kernel queues 32 unaccepted connections per listener -/// (`listener::MAX_PENDING_CONNECTIONS`); this is the same allowance one step -/// further along, for a client that has been accepted and has not yet said what -/// it wants. Past it netstack refuses by name rather than growing, and -/// [`HANDSHAKE_TIMEOUT`] is what guarantees the table drains. -const MAX_PENDING_CONNS: u32 = 32; - -/// How long an accepted connection may go without completing its request. -/// -/// Policy, and generous: every client in the tree sends its request in the -/// statement after `connect` (`toyos::net`'s `NetstackConn::request`). What this -/// bounds is the one that never sends it. -const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(2); - -/// The largest request payload netstack keeps. -/// -/// `MsgType::DnsLookup` is the only request carrying bytes rather than a struct, -/// and `toyos::net::dns_lookup` frames a hostname into a 256-byte buffer; every -/// typed request is far smaller, `TcpConnectPipedRequest` at 32 bytes being the -/// widest. A client may declare anything up to `ipc::MAX_FRAME_LEN` — the excess -/// is counted down and discarded, never waited for. -const MAX_KEPT_REQUEST: usize = 256; - /// Registrations one piped connection can make in a batch: its send pipe and /// its receive pipe. const POLL_HANDLES_PER_PIPED: u32 = 2; @@ -608,42 +376,6 @@ const LOOKUP_POLL_HANDLES: u32 = resolve::MAX_LOOKUPS as u32; const MAX_PIPED_SLOTS: u64 = ((Poller::MAX_HANDLES - FIXED_POLL_HANDLES - MAX_PENDING_CONNS - LOOKUP_POLL_HANDLES) / POLL_HANDLES_PER_PIPED) as u64; -/// One client's inbound framing. -/// -/// **netstack never reads a client with a blocking read.** That is the whole point -/// of [`ipc::FrameRx`]: `ipc::recv_header` and `ipc::recv_payload` park the -/// caller until the peer sends the bytes it promised. Here a peer that stops halfway -/// through a frame costs a buffer and a deadline instead of the event loop. -type ClientRx = ipc::FrameRx; - -/// A connection that has been accepted and has not yet said what it wants. -/// -/// It exists because `accept` and the request frame are two events. -struct PendingConn { - conn: Connection, - rx: ClientRx, - since: Instant, -} - -/// A whole request, off the connection and in memory. -/// -/// The payload travels with the frame instead of being read off the connection -/// during -/// dispatch: the read side is finished before anything acts on a message, so no -/// handler below can park on the client that sent it. -struct Request { - client: Client, - msg_type: u32, - payload: [u8; MAX_KEPT_REQUEST], - payload_len: usize, -} - -impl Request { - fn payload(&self) -> &[u8] { - &self.payload[..self.payload_len] - } -} - /// Payload bytes a UDP socket's receive buffer holds, and therefore the longest /// datagram netstack can ever hand back — which is what bounds the buffer /// [`Netstack::deliver_datagram`] sizes from a client's `max_len`. From 6d2dd8bb224c13636d55cbeb57cc5748e3619e45 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 22:24:22 +0200 Subject: [PATCH 2/7] Both NIC drivers say their transmit room and wake netstack when it returns A frame is no longer written into a scratch buffer and dropped when the transmit ring is full. `toyos_i219::I219::tx_room` counts the room from `tx_next` and `tx_clean` after a reclaim; `wake_on_room` unmasks the transmit-done cause (`TXDW`, and `TXQ0` for a part in MSI-X mode), counts again, and `begin_pass` masks it. Unmasked first and counted after, so a write-back between a count of zero and the unmasking is not waited on. virtio's room is its free heads after a reclaim; its device already interrupts for every used transmit buffer, so there is nothing to arm. netstack's `DmaNic` answers smoltcp `None` from `transmit` with no room, and from `receive` too, because smoltcp takes a transmit token with every frame it receives. smoltcp keeps what it could not send and asks "now" on every pass, so the loop leaves smoltcp's deadline out of its wait while the card has no room: the card's claim begins the pass that can send. Deleted: both scratch buffers, `Counters::tx_dropped`, virtio's drop count, and the two clauses of the report lines that no frame can reach any more (a frame offered with no room, or longer than a buffer, is netstack's own bug and panics by name). `issues/netstacks-i219-drops-a-transmit-burst-past-its-ring.md` closes: its exit was `transmit` answering `None` and a host test pushing more than a ring through without a drop, which is `a_burst_past_the_ring_leaves_whole_on_the_room_it_is_told` at the driver. `issues/netstacks-transmit-drop-report-cadence-has-no-deterministic-test.md` closes with its subject: no driver counts a transmit drop, so none can report one from `tx`. What no hardware has read of the new path is filed as `issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...19-drops-a-transmit-burst-past-its-ring.md | 39 ---- ...eport-cadence-has-no-deterministic-test.md | 33 --- ...cs-room-and-wake-are-unread-on-hardware.md | 35 ++++ toyos-i219/src/lib.rs | 79 +++++-- toyos-i219/src/regs.rs | 9 + toyos-i219/src/stub.rs | 6 + toyos-i219/src/tests.rs | 196 +++++++++++++++--- userland/netstack/src/card.rs | 24 ++- userland/netstack/src/i219.rs | 46 ++-- userland/netstack/src/main.rs | 27 ++- userland/netstack/src/virtio_net.rs | 51 +++-- 11 files changed, 369 insertions(+), 176 deletions(-) delete mode 100644 issues/netstacks-i219-drops-a-transmit-burst-past-its-ring.md delete mode 100644 issues/netstacks-transmit-drop-report-cadence-has-no-deterministic-test.md create mode 100644 issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md diff --git a/issues/netstacks-i219-drops-a-transmit-burst-past-its-ring.md b/issues/netstacks-i219-drops-a-transmit-burst-past-its-ring.md deleted file mode 100644 index a6beb7a3cc6..00000000000 --- a/issues/netstacks-i219-drops-a-transmit-burst-past-its-ring.md +++ /dev/null @@ -1,39 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-29 ---- - -# netd's I219 drops a transmit burst past its ring, and only a retransmit timeout recovers it - -`I219::tx` (`userland/netd/src/i219.rs`) writes a frame the driver has no -descriptor for into a scratch buffer and drops it: smoltcp's `TxToken` cannot -refuse. But `Device::transmit` can, by answering `None`, and then smoltcp keeps -the bytes in the socket and sends them on a later poll. netd's `transmit` -(`userland/netd/src/main.rs`) answers `Some` every time. `toyos_i219::TX_RING` -is 16, so 15 frames can be in flight. Every frame after those 15 in one poll is -lost, and TCP gets it back only when its retransmission timeout fires. For -smoltcp 0.12 that timeout starts at 700 ms (`RTTE_INITIAL_RTT` + -4 × `RTTE_INITIAL_DEV`). - -## Measured - -The T14 run of PR #609 at `7d2e15c0`, boot `lantalkcase`. `logd` admitted the -host's stream at 19.141 s. At 19.180 s netd said `9 frame(s) dropped with no -transmit descriptor free`, and `reboot` came at 19.403 s. The host's -`stream.log` is `kernel.log`'s first 217 lines, which is 21890 bytes, and -15 × 1460 = 21900 falls inside line 218. The run of `main` at `7e151819` -stopped at the same 217th line, with 595 ms between the admit and the stop. -Both windows are shorter than the first retransmit timeout. The same count appears -on the `lanswapcase` boot: 11, 18 and 23 frames dropped. - -## Owner - -Stage 2 of `issues/the-lan-is-not-yet-production-grade.md`, a gigabit -driver that holds. - -## Exit - -`transmit` answers `None` when no descriptor is free, and a descriptor coming -free wakes netd's poll. A host test pushes more than `TX_RING` frames' worth -through one poll and asserts that `tx_dropped` stays 0 and every byte leaves. diff --git a/issues/netstacks-transmit-drop-report-cadence-has-no-deterministic-test.md b/issues/netstacks-transmit-drop-report-cadence-has-no-deterministic-test.md deleted file mode 100644 index 0dcf363c1d5..00000000000 --- a/issues/netstacks-transmit-drop-report-cadence-has-no-deterministic-test.md +++ /dev/null @@ -1,33 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-09-25 ---- - -# netd's transmit-drop report cadence has no test that reds on a line per drop - -netd says what its NIC counted once a pass, after the pass's `iface.poll` -loop, and never from a driver's `tx`: T14 run 130's `/log` carried the I219's -counter line once per dropped frame, the drop count going 1, 2, 3 ... at one -timestamp, while logd served its backlog to the host. Nothing in the suite -stages a burst of drops, so a driver that reports from `tx` again passes every -test. - -Measured in QEMU, `tests/logstreamcase` with a reader served -`test_rs_log_flood`'s five megabytes: QEMU's virtio-net takes transmitted -frames in a bottom half of its main loop, and the guest's queue of 16 fills -only when the host deschedules that loop. Dropped frames, by host load (the -harness's own width factor): 330 at 3.02x with `x-txburst=1`, 40 at 1.34x -with the default device, and 0 twice at 1.30x with `x-txburst=1`. Every -QEMU stall of the transmit path that holds the BQL also blocks the guest's -doorbell, so netd waits instead of dropping. A guest-side hold (the driver -taking heads back once a pass) dropped nothing: logd's data reaches netd's -socket a few segments a pass. - -What would stage it on any host: a `-netdev stream` whose host end stops -reading, which makes virtio-net wait on its peer with the guest running, beside -a host-side DHCP responder so netd has an address to send from; or a netd -seam that drives the virtio driver's `tx` over plain memory in a host test. - -Exit: a test that is red with the report called from `tx` and green with it -called once a pass, shown on a tree that builds. diff --git a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md new file mode 100644 index 00000000000..de2b08a14f2 --- /dev/null +++ b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md @@ -0,0 +1,35 @@ +--- +status: open +kind: tooling +opened: 2026-10-08 +--- + +# The Intel NIC's transmit room and its wake have been read on no hardware + +netstack offers its card a frame only when the driver counts room for it, and +where there is none it sleeps on the claim until `toyos_i219::I219::wake_on_room`'s +cause arrives. On the I219 and the 82574 every reading of that path is the +driver's own model (`toyos-i219/src/stub.rs`), which is the 82574 datasheet +written down: no image gives netstack `pci:8086:15fc`, and the harness has no +Intel card, so neither the T14 nor QEMU's `e1000e` has run it. + +What the model cannot say of the T14's part: which of the cause's two names +(`TXDW`, or `TXQ0` through `IVAR`) the PCH's MAC raises a message for, and +whether a ring left full across a link going down is written back when the +link returns. + +A wake that never comes is no wedge and no lost frame: room is counted from +the descriptors in memory, so the next pass any other event begins finds it. +It is a stall until that pass. + +## Owner + +The T14's outbound rows of `issues/toyos-has-its-own-network-stack.md`: the +first boot that gives netstack the wired card. + +## Exit + +A T14 boot whose netstack drives `8086:15fc` sends more than +`toyos_i219::TX_RING - 1` frames in one pass, and its readback shows the pass +after the full ring begun by the card's interrupt and every frame sent +(`descriptors.sent` against `wire.sent` in `inspect`). diff --git a/toyos-i219/src/lib.rs b/toyos-i219/src/lib.rs index 4c818cc3d9f..13ee40efb5a 100644 --- a/toyos-i219/src/lib.rs +++ b/toyos-i219/src/lib.rs @@ -31,6 +31,13 @@ //! interrupt record. On the host they are `stub.rs`, which is the datasheet //! written down. //! +//! # A frame is offered only to a ring with room for it +//! +//! [`I219::tx_room`] is asked before there is a frame, and a caller answered +//! none sleeps on its claim once [`I219::wake_on_room`] has unmasked the one +//! cause that says a descriptor came back. Nothing here drops a frame for want +//! of a descriptor, and nothing waits on the part. +//! //! # The device is not trusted //! //! Every number in a written-back descriptor is the device's, and this driver @@ -166,8 +173,9 @@ pub trait Interrupts { /// multiple of 128", so the count is a multiple of eight. pub const RX_RING: usize = 256; /// How many transmit descriptors the ring holds. A frame is handed to the -/// device the moment it is filled and reclaimed on the next send, so what this -/// bounds is how many may be in flight at once. +/// device the moment it is filled and reclaimed when room is next asked for, +/// so what this bounds is how many may be in flight at once: one fewer than +/// this, [`I219::tx_room`]'s most. pub const TX_RING: usize = 16; /// Bytes per receive buffer — `RCTL.BSIZE = 00b` with `BSEX` clear (§10.2.5.1). pub const RX_BUF_BYTES: usize = 2048; @@ -401,10 +409,6 @@ pub struct Counters { pub starved: u32, /// Frames the caller offered that no transmit buffer holds. pub too_long: u32, - /// Frames dropped for want of a free transmit descriptor. A count and not - /// a wait: a server never blocks, and a dropped frame's recovery is the - /// peer's retransmit. - pub tx_dropped: u32, /// Messages that arrived with no cause set in `ICR` — §7.4.5's spurious /// interrupt. It costs a pass and nothing else. pub spurious: u32, @@ -713,6 +717,9 @@ pub struct I219 { tx_next: usize, /// Next transmit descriptor to reclaim. tx_clean: usize, + /// Whether [`cause::TX_DONE`] is unmasked: a caller found the ring full + /// and no pass has begun since. + tx_wake: bool, counters: Counters, /// What [`Self::wire`] has read out of the statistics registers so far. wire: Wire, @@ -844,6 +851,7 @@ impl I219 { rx_budget: RX_BUDGET, tx_next: 0, tx_clean: 0, + tx_wake: false, counters: Counters::default(), wire: Wire::default(), }; @@ -1026,6 +1034,15 @@ impl I219 { Err(why) => return Err(why), }; + // §10.2.4.6: the transmit cause is masked again, because it was wanted + // for one wake and left unmasked it is a message for every frame sent. + // Before the room this pass's sends ask for: a ring still full arms it + // again in [`Self::wake_on_room`]. + if self.tx_wake { + self.regs.write(regs::IMC, cause::TX_DONE); + self.tx_wake = false; + } + // Once, and written back. §10.2.4.1's case 3 says a read with no // interrupt asserted has no side effect at all, so a driver that // treated the read as the acknowledgement would see the same causes @@ -1169,25 +1186,51 @@ impl I219 { self.regs.write(regs::RDT, tail as u32); } - /// Take a transmit descriptor and its buffer, or `None` where every one is - /// in flight or the frame does not fit one. + /// How many frames the transmit ring takes now, every descriptor the part + /// has written back taken first. + /// + /// §7.2.4: hardware owns `[TDH..TDT)`, so a ring filled to the last + /// descriptor would wrap the tail onto the head and read as empty — one + /// descriptor is never handed out, and a full ring is [`TX_RING`]` - 1` in + /// flight. + pub fn tx_room(&mut self) -> usize { + self.reclaim_tx(); + (self.tx_clean + TX_RING - 1 - self.tx_next) % TX_RING + } + + /// Ask for a message when the part next writes a transmit descriptor + /// back, and answer the room there is now that it has been asked. + /// + /// §10.2.4.5: a write of [`cause::TX_DONE`] to `IMS` unmasks it, and it + /// stays unmasked until the next [`Self::begin_pass`]. /// - /// Non-blocking by construction. A caller with no slot drops the frame: - /// spinning on the ring here would park its whole event loop on a device. + /// **Unmasked first and counted after**, so no write-back is lost between + /// the two: one that landed before the mask was written raised nothing and + /// is in the count, and one that lands after it is a message. A caller + /// answered 0 waits on its claim. + pub fn wake_on_room(&mut self) -> usize { + if !self.tx_wake { + self.regs.write(regs::IMS, cause::TX_DONE); + self.tx_wake = true; + } + self.tx_room() + } + + /// Take a transmit descriptor and its buffer, or `None` where the ring has + /// no room ([`Self::tx_room`]) or the frame does not fit a buffer. + /// + /// Non-blocking by construction: a caller asks for room before it has a + /// frame, and waits on [`Self::wake_on_room`] where there is none. pub fn tx_reserve(&mut self, len: usize) -> Option { // §7.2.10.1: one legacy descriptor carries one buffer, and this // driver's is `TX_BUF_BYTES`. A longer frame is refused rather than - // truncated into one, and counted apart from a full ring because it is - // a caller that offered more than it was told it could. + // truncated into one, and counted because it is a caller that offered + // more than it was told it could. if len > TX_BUF_BYTES { self.counters.too_long = self.counters.too_long.saturating_add(1); return None; } - self.reclaim_tx(); - // §7.2.4: hardware owns `[TDH..TDT)`, so a ring filled to the last - // descriptor would wrap the tail onto the head and read as empty. - if (self.tx_next + 1) % TX_RING == self.tx_clean { - self.counters.tx_dropped = self.counters.tx_dropped.saturating_add(1); + if self.tx_room() == 0 { return None; } let index = self.tx_next; @@ -1232,7 +1275,7 @@ impl I219 { /// Take back every transmit descriptor the part has finished with, so /// [`Counters::sent`] says what has left and not only what was handed /// over — for a caller about to report it, since the ring is otherwise - /// reclaimed only when the next frame needs a slot. + /// reclaimed only when room is asked for. pub fn reclaim(&mut self) { self.reclaim_tx(); } diff --git a/toyos-i219/src/regs.rs b/toyos-i219/src/regs.rs index 92cecd1bc59..8e590e0a942 100644 --- a/toyos-i219/src/regs.rs +++ b/toyos-i219/src/regs.rs @@ -381,6 +381,15 @@ pub mod cause { /// when every cause has. pub const INT_ASSERTED: u32 = 1 << 31; + /// A transmit descriptor written back, under both of §10.2.4.1's names for + /// it: `TXDW`, and the `TXQ0` §10.2.4.9's `IVAR` gives a vector, which is + /// the only one of the two a part in MSI-X mode raises anything for. + /// §7.2.4.2: the write-back is of a descriptor that carried `RS`, which + /// every one this driver publishes does, and with `IDE` clear no timer + /// holds the cause back. **Not in [`ENABLED`]**: it is unmasked while a + /// caller waits on a full transmit ring, and never otherwise. + pub const TX_DONE: u32 = TXDW | TXQ0; + /// What §4.6.5 tells a driver to unmask: "Suggested bits include RXT, RXO, /// RXDMT and LSC. There is no reason to enable the transmit interrupts." pub const ENABLED: u32 = RXT0 | RXO | RXDMT0 | LSC; diff --git a/toyos-i219/src/stub.rs b/toyos-i219/src/stub.rs index 5b2abf517c8..fbbb3278fc1 100644 --- a/toyos-i219/src/stub.rs +++ b/toyos-i219/src/stub.rs @@ -2321,6 +2321,12 @@ impl Nic { core::mem::take(&mut self.0.borrow_mut().sent) } + /// Messages the claim holds that no pass has taken: what a caller asleep + /// on its claim would be woken by. + pub fn pending(&self) -> u32 { + self.0.borrow().messages + } + /// Stop the device acting on a tail register write, so it acts only when /// [`Self::run`] says so. pub fn hold(&self) { diff --git a/toyos-i219/src/tests.rs b/toyos-i219/src/tests.rs index 654d8a99aea..2771fad2e38 100644 --- a/toyos-i219/src/tests.rs +++ b/toyos-i219/src/tests.rs @@ -604,31 +604,175 @@ fn a_frame_goes_out_whole() { assert_eq!(nic.sent(), vec![payload], "{}", nic.because("the frame did not reach the wire")); } -/// **A server never blocks.** With every descriptor in flight the next frame is -/// dropped and counted, never waited on: spinning here would park the caller's -/// whole event loop on a device. +/// Publish frames tagged from `first` until the driver refuses one, and answer +/// them. The device is held, so nothing is written back while this runs. +fn fill(nic: &Nic, driver: &mut Driver, first: u8) -> Vec> { + let mut out = Vec::new(); + while let Some(slot) = driver.tx_reserve(64) { + let payload = frame(first + out.len() as u8, 64); + nic.put_bytes(slot.at, &payload); + driver.tx_commit(slot); + out.push(payload); + if out.len() > TX_RING * 2 { + panic!("{}", nic.because("the transmit ring never filled")); + } + } + out +} + +/// Let the held device work until every descriptor it was given is back. +fn settle(nic: &Nic, driver: &mut Driver) { + for _ in 0..TX_RING * 4 { + if driver.tx_room() == TX_RING - 1 { + return; + } + nic.run(); + } + panic!("{}", nic.because("the transmit ring never emptied")); +} + +/// §7.2.4: hardware owns `[TDH..TDT)`, so filling the last descriptor would +/// wrap the tail onto the head and the part would read a full ring as an empty +/// one. The ring takes one frame fewer than it has descriptors, says so before +/// a frame is offered, and refuses the next. #[test] -fn a_full_transmit_ring_drops_rather_than_waits() { +fn a_full_transmit_ring_answers_room_0() { let nic = Nic::new(11); let mut driver = open(&nic); nic.set_link(true); - // The device is held, so nothing is sent, nothing is written back and - // nothing is reclaimed: the ring really does fill. + // Nothing is sent, written back or reclaimed until `run`: the ring fills. nic.hold(); - // The ring keeps one descriptor in software's hands (§7.2.4: hardware owns - // `[TDH..TDT)`, so filling the last one would wrap the tail onto the head). - let mut taken = 0; - while let Some(slot) = driver.tx_reserve(64) { - nic.put_bytes(slot.at, &frame(1, 64)); - driver.tx_commit(slot); - taken += 1; - if taken > TX_RING * 2 { - panic!("{}", nic.because("the transmit ring never filled")); + assert_eq!(driver.tx_room(), TX_RING - 1, "{}", nic.because("an empty ring is not the ring less one")); + let out = fill(&nic, &mut driver, 1); + assert_eq!(out.len(), TX_RING - 1, "{}", nic.because("the usable depth is not the ring less one")); + assert_eq!(driver.tx_room(), 0, "{}", nic.because("a full ring said it had room")); + assert!(driver.tx_reserve(64).is_none()); + nic.run(); + assert_eq!(nic.sent(), out, "{}", nic.because("the part did not read the full ring as full")); +} + +/// §7.2.4.2: a descriptor that carried `RS` is written back with `DD`, and +/// that is the buffer coming free. Room is counted from memory and returns +/// with the write-back, by as many frames as were written back. +#[test] +fn a_written_back_descriptor_returns_room() { + let permits = Permits { batched_writeback: false, ..Permits::default() }; + let nic = Nic::with(23, Part::E82574, permits); + let mut driver = open(&nic); + nic.set_link(true); + nic.hold(); + fill(&nic, &mut driver, 1); + assert_eq!(driver.tx_room(), 0); + nic.run(); + assert_eq!( + driver.tx_room(), + TX_RING - 1, + "{}", + nic.because("every descriptor was written back and the room did not return") + ); + assert_eq!(driver.counters().sent, (TX_RING - 1) as u32); +} + +/// §4.6.5: "There is no reason to enable the transmit interrupts" — until a +/// caller has a frame and no descriptor. The cause is unmasked by the caller +/// that found the ring full, raises the one message that ends its wait, and is +/// masked again by the pass that takes it: a part left with it unmasked sends a +/// message for every frame. +#[test] +fn the_transmit_cause_is_armed_only_when_asked() { + let permits = Permits { spurious_interrupts: false, ..Permits::default() }; + let nic = Nic::with(24, Part::E82574, permits); + let mut driver = open(&nic); + nic.set_link(true); + one_pass(&mut driver); + nic.hold(); + + // Not asked: the ring fills and empties, and nothing is said about it. + fill(&nic, &mut driver, 1); + nic.run(); + assert_eq!(nic.peek(regs::IMS) & cause::TX_DONE, 0); + assert_eq!( + nic.pending(), + 0, + "{}", + nic.because("a write-back nobody waited on sent a message") + ); + settle(&nic, &mut driver); + one_pass(&mut driver); + + // Asked, on a full ring. + fill(&nic, &mut driver, 0x40); + assert_eq!(driver.wake_on_room(), 0); + assert_eq!( + nic.peek(regs::IMS), + cause::ENABLED | cause::ENABLED_MSIX | cause::TX_DONE, + "{}", + nic.because("the transmit cause was not unmasked under both its names") + ); + assert_eq!(nic.pending(), 0); + nic.run(); + assert!(nic.pending() > 0, "{}", nic.because("a write-back the caller waited on sent no message")); + let pass = one_pass(&mut driver); + assert_eq!(pass.causes & cause::TX_DONE, cause::TX_DONE); + assert!(driver.tx_room() > 0, "{}", nic.because("the message came and the room did not")); + + // And not asked again: the pass that took the message masked the cause. + assert_eq!( + nic.peek(regs::IMS), + cause::ENABLED | cause::ENABLED_MSIX, + "{}", + nic.because("the pass left the transmit cause unmasked") + ); + settle(&nic, &mut driver); + assert_eq!( + nic.pending(), + 0, + "{}", + nic.because("a write-back after the wake was taken sent a message") + ); +} + +/// A burst several rings long, sent by a caller that asks for room before each +/// frame and sleeps on its claim where there is none: every frame leaves, in +/// order, and every sleep is ended by a message. +#[test] +fn a_burst_past_the_ring_leaves_whole_on_the_room_it_is_told() { + for seed in 1..=32u64 { + let nic = Nic::new(seed); + let mut driver = open(&nic); + nic.set_link(true); + one_pass(&mut driver); + nic.hold(); + let mut out: Vec> = Vec::new(); + let mut on_the_wire: Vec> = Vec::new(); + let mut sleeps = 0; + for i in 0..(TX_RING as u8 * 4) { + while driver.tx_room() == 0 && driver.wake_on_room() == 0 { + // Asleep on the claim: the device works, and nothing but a + // message begins the next pass. + let mut runs = 0; + while nic.pending() == 0 { + nic.run(); + on_the_wire.extend(nic.sent()); + runs += 1; + assert!(runs < 64, "{}", nic.because("no message ever came for a full ring")); + } + one_pass(&mut driver); + sleeps += 1; + } + let payload = frame(i + 1, 100 + i as usize); + let slot = driver.tx_reserve(payload.len()).expect("the room the driver just counted"); + nic.put_bytes(slot.at, &payload); + driver.tx_commit(slot); + out.push(payload); } - // Nothing runs, so nothing is written back and nothing is reclaimed. + for _ in 0..TX_RING { + nic.run(); + on_the_wire.extend(nic.sent()); + } + assert!(sleeps > 0, "{}", nic.because("the burst never filled the ring")); + assert_eq!(on_the_wire, out, "{}", nic.because("what reached the wire is not the burst")); } - assert_eq!(taken, TX_RING - 1, "{}", nic.because("the usable depth is not the ring less one")); - assert_eq!(driver.counters().tx_dropped, 1); } /// §7.2.4.2 lets the device hold transmit write-backs in a batch, so a later @@ -667,12 +811,6 @@ fn transmit_descriptors_are_reclaimed_under_batched_write_back() { "{}", nic.because("what reached the wire is not what was published") ); - assert_eq!( - driver.counters().tx_dropped, - 0, - "{}", - nic.because("a frame was dropped although the ring was being reclaimed") - ); } } @@ -839,8 +977,8 @@ fn the_receiver_reporting_on_itself_is_counted() { } /// §7.2.10.1: one legacy descriptor carries one buffer. A frame longer than one -/// is refused and counted apart from a full ring, because it is a caller that -/// offered more than it was told it could. +/// is refused and counted, because it is a caller that offered more than it +/// was told it could. #[test] fn a_frame_longer_than_a_transmit_buffer_is_refused_and_not_truncated() { let nic = Nic::new(22); @@ -848,12 +986,6 @@ fn a_frame_longer_than_a_transmit_buffer_is_refused_and_not_truncated() { nic.set_link(true); assert!(driver.tx_reserve(TX_BUF_BYTES + 1).is_none()); assert_eq!(driver.counters().too_long, 1); - assert_eq!( - driver.counters().tx_dropped, - 0, - "{}", - nic.because("a frame that never fit was counted as a full ring") - ); // And the ring is untouched: the next frame that does fit still goes out. let payload = frame(0x99, TX_BUF_BYTES); let slot = driver.tx_reserve(payload.len()).expect("a free transmit descriptor"); diff --git a/userland/netstack/src/card.rs b/userland/netstack/src/card.rs index 55ad26666a4..25f2c8d68f9 100644 --- a/userland/netstack/src/card.rs +++ b/userland/netstack/src/card.rs @@ -125,6 +125,28 @@ impl Card { snap.put("errors.crc", wire.crc_errors); } + /// How many frames the card takes now. [`Self::tx`] is for a caller this + /// answered: a card with no room is not offered a frame. + pub fn tx_room(&self) -> usize { + match self { + Self::Virtio(nic) => nic.tx_room(), + Self::Intel(nic) => nic.tx_room(), + } + } + + /// Have the claim read ready when room returns, and answer the room there + /// is once that is so: a caller answered 0 has nothing to do before the + /// claim wakes it. + /// + /// virtio is asked nothing: its every finished transmit interrupts + /// (`VirtioNet::tx_room`). + pub fn wake_on_room(&self) -> usize { + match self { + Self::Virtio(nic) => nic.tx_room(), + Self::Intel(nic) => nic.wake_on_room(), + } + } + pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { match self { Self::Virtio(nic) => nic.tx(len, fill), @@ -133,7 +155,7 @@ impl Card { } /// Say what the driver counted, once a pass and after every frame the pass - /// sent: a line per dropped frame is itself more frames to send. + /// sent: a line per refused descriptor is itself more frames to send. pub fn report(&self) { match self { Self::Virtio(nic) => nic.report(), diff --git a/userland/netstack/src/i219.rs b/userland/netstack/src/i219.rs index 72968ae012a..48914e85e6a 100644 --- a/userland/netstack/src/i219.rs +++ b/userland/netstack/src/i219.rs @@ -141,10 +141,6 @@ pub struct Nic { claim: Rc, /// netstack's view of the same grant: the frame payloads, and nothing else. frames: Window, - /// Where a dropped frame is written. Ordinary memory outside the grant, so - /// no device can reach it: smoltcp's token has to be given somewhere to put - /// its bytes even when there is no descriptor to send them on. - dropped: RefCell>, mac: [u8; 6], part: toyos_i219::Part, reported: Latch<(toyos_i219::Counters, toyos_i219::Link)>, @@ -257,7 +253,6 @@ impl Nic { driver: RefCell::new(driver), claim: dev, frames, - dropped: RefCell::new(vec![0; toyos_i219::TX_BUF_BYTES]), mac, part, reported: Latch::default(), @@ -321,21 +316,27 @@ impl Nic { self.driver.borrow_mut().rx_done(frame); } + /// How many frames the transmit ring takes now. + pub fn tx_room(&self) -> usize { + self.driver.borrow_mut().tx_room() + } + + /// Have the part interrupt when a transmit descriptor comes back, and + /// answer the room there is once it has been told to. + pub fn wake_on_room(&self) -> usize { + self.driver.borrow_mut().wake_on_room() + } + /// Fill a transmit buffer with a `len`-byte frame and hand it to the /// device. /// - /// Non-blocking. A frame the driver has no slot for is written into the - /// scratch buffer and dropped — **a server never blocks**, smoltcp's token - /// cannot say no, and a dropped frame's recovery is the peer's retransmit. + /// Non-blocking, and for a caller [`Self::tx_room`] answered: a frame the + /// driver has no slot for is a caller that did not ask, or one that offered + /// more than a buffer holds. pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { - let slot = self.driver.borrow_mut().tx_reserve(len); - let Some(slot) = slot else { - let mut scratch = self.dropped.borrow_mut(); - if scratch.len() < len { - scratch.resize(len, 0); - } - return fill(&mut scratch[..len]); - }; + let slot = self.driver.borrow_mut().tx_reserve(len).unwrap_or_else(|| { + panic!("netstack: a {len}-byte frame was offered to a transmit ring that refuses it") + }); let window = self.frames.sub(slot.at, len); // SAFETY: the window is inside the grant, which lives as long as // `self`; the device is not reading it, because this descriptor is out @@ -346,10 +347,10 @@ impl Nic { result } - /// Say what this driver has refused, dropped or been told about, and what - /// the link is doing — when either has moved. Once a pass, never per frame: - /// a burst of drops is one line, where a line per drop is more frames for a - /// served log to send through the ring that is already full. + /// Say what this driver has refused or been told about, and what the link + /// is doing — when either has moved. Once a pass, never per frame: a burst + /// of refusals is one line, where a line each is more frames for a served + /// log to send. /// /// Keyed on the anomalies and never on the spurious count: that one moves /// on its own. @@ -379,16 +380,13 @@ impl Nic { crate::say!( "netstack: I219: refused {} over-length, {} errored, {} split and {} empty \ descriptor(s); {} overrun(s), {} descriptor-starvation report(s), \ - {} frame(s) too long for a transmit buffer, {} frame(s) dropped with no \ - transmit descriptor free, {} spurious interrupt(s)", + {} spurious interrupt(s)", counters.over_length, counters.errored, counters.split, counters.empty, counters.overruns, counters.starved, - counters.too_long, - counters.tx_dropped, counters.spurious, ); } diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index 2a7b25a436d..7a0afafdde4 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -64,11 +64,24 @@ struct DmaNic { nic: Card, } +impl DmaNic { + /// Whether the card takes a frame now. Where it does not, its claim reads + /// ready when it will. + fn room(&self) -> bool { + self.nic.tx_room() > 0 || self.nic.wake_on_room() > 0 + } +} + impl Device for DmaNic { type RxToken<'a> = DmaRxToken<'a>; type TxToken<'a> = DmaTxToken<'a>; fn receive(&mut self, _timestamp: SmoltcpInstant) -> Option<(Self::RxToken<'_>, Self::TxToken<'_>)> { + // A frame is taken off the receive ring only with room to answer it: + // smoltcp takes a transmit token with every frame it receives. + if !self.room() { + return None; + } let token = match &self.nic { Card::Virtio(nic) => { nic.poll_rx().map(|(index, len)| DmaRxToken::Virtio { nic, index, len }) @@ -79,7 +92,7 @@ impl Device for DmaNic { } fn transmit(&mut self, _timestamp: SmoltcpInstant) -> Option> { - Some(DmaTxToken { nic: &self.nic }) + self.room().then_some(DmaTxToken { nic: &self.nic }) } fn capabilities(&self) -> DeviceCapabilities { @@ -1494,8 +1507,16 @@ fn main() { // delayed ACK — and zero when it has a frame to send now. A piped // connection needs nothing else: its peer's bytes wake the NIC, and its // client's bytes and room wake the watches below. - let smoltcp_due = - iface.poll_delay(now, &socket_set).map_or(u64::MAX, |d| d.total_micros().saturating_mul(1000)); + // + // **None of them while the card has no room.** Each is a frame to send + // or to take, the card refuses both, and smoltcp's "now" would be a + // pass every time round until it stops; the card's claim begins the + // pass that can. + let smoltcp_due = if device.room() { + iface.poll_delay(now, &socket_set).map_or(u64::MAX, |d| d.total_micros().saturating_mul(1000)) + } else { + u64::MAX + }; // A pending UDP receive or connect has no wake of its own. let has_pending_async = !daemon.pending_udp_recvs.is_empty() diff --git a/userland/netstack/src/virtio_net.rs b/userland/netstack/src/virtio_net.rs index d656fcb1202..5f31eaff840 100644 --- a/userland/netstack/src/virtio_net.rs +++ b/userland/netstack/src/virtio_net.rs @@ -17,7 +17,7 @@ //! Virtio 1.2 throughout: §4.1.4 for the PCI capability layout, §2.7 for the //! split virtqueue, §5.1 for the network device. -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use toyos::shm::SharedMemory; use toyos::volatile::Window; @@ -352,14 +352,7 @@ pub struct VirtioNet { tx: RefCell, /// Transmit heads nothing is in flight on. tx_free: RefCell>, - /// Frames dropped for want of one. A count and not a wait: a server never - /// blocks, and a dropped frame's recovery is the peer's retransmit. - tx_dropped: Cell, - /// Where a dropped frame is written. Ordinary memory outside the grant, so - /// no device can reach it: smoltcp's token has to be given somewhere to put - /// its bytes even when there is no head to send them on. - dropped: RefCell>, - reported: Latch<(u32, u32)>, + reported: Latch, mac: [u8; 6], } @@ -525,8 +518,6 @@ impl VirtioNet { rx: RefCell::new(rx), tx: RefCell::new(tx), tx_free: RefCell::new((0..TX_QUEUE_SIZE).rev().collect()), - tx_dropped: Cell::new(0), - dropped: RefCell::new(vec![0; TX_BUF_SIZE]), reported: Latch::default(), mac, }; @@ -542,18 +533,16 @@ impl VirtioNet { self.mac } - /// Say what this driver has refused or dropped, when either count has - /// moved. Once a pass, never per frame: a burst of drops is one line. + /// Say what this driver has refused, when the count has moved. Once a + /// pass, never per element: a burst of refusals is one line. pub fn report(&self) { let refused = self.rx.borrow().refused + self.tx.borrow().refused; - let dropped = self.tx_dropped.get(); - if self.reported.moved((refused, dropped)).is_none() { + if self.reported.moved(refused).is_none() { return; } crate::say!( "netstack: this NIC has refused {refused} used-ring element(s) — the device named a \ - descriptor this driver never published or claimed more bytes than it was given — \ - and dropped {dropped} frame(s) with no transmit descriptor free" + descriptor this driver never published or claimed more bytes than it was given" ); } @@ -627,6 +616,18 @@ impl VirtioNet { unsafe { std::slice::from_raw_parts(window.as_ptr() as *const u8, len) } } + /// How many frames the transmit queue takes now, every head the device + /// has finished with taken back first. + /// + /// **Room returning is an interrupt already**: this driver negotiates no + /// `VIRTIO_F_EVENT_IDX` and leaves the transmit queue's `avail.flags` 0, + /// and §2.7.7 has the device notify for every buffer it uses on such a + /// queue. A caller answered 0 sleeps on the claim. + pub fn tx_room(&self) -> usize { + self.reclaim_tx(); + self.tx_free.borrow().len() + } + /// Fill a transmit buffer with a `len`-byte frame and hand it to the device. /// /// **The buffer is the head's own and the two are taken together**, so @@ -634,20 +635,18 @@ impl VirtioNet { /// `tx_free` only here and comes back only in [`Self::reclaim_tx`], which /// the device's used ring is what drives. /// - /// Non-blocking. A frame with no head free is written into the scratch - /// buffer and dropped — **a server never blocks**, smoltcp's token cannot - /// say no, and a dropped frame's recovery is the peer's retransmit; - /// spinning on the used ring here would park netstack on a device. + /// Non-blocking, and for a caller [`Self::tx_room`] answered: a frame + /// offered with no head free is a caller that did not ask. pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { assert!( NET_HDR_SIZE + len <= TX_BUF_SIZE, "netstack: a {len}-byte frame does not fit this NIC's transmit buffer" ); - self.reclaim_tx(); - let Some(head) = self.tx_free.borrow_mut().pop() else { - self.tx_dropped.set(self.tx_dropped.get().saturating_add(1)); - return fill(&mut self.dropped.borrow_mut()[..len]); - }; + let head = self + .tx_free + .borrow_mut() + .pop() + .expect("netstack: a frame was offered to a transmit queue that had said it has no room"); let at = OFF_TX_BUFS + head as usize * TX_BUF_SIZE; // The header is this driver's and zeroed before the frame goes in. self.dma.sub(at, NET_HDR_SIZE).zero(); From 12e4fb26926384bc643d0a3eac921c50eb4c1b7b Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 23:30:30 +0200 Subject: [PATCH 3/7] Review round 1 of #782: the transmit cause per part, a ring the link change left is taken back, and the wait is counted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read against Intel's own documents this round (82574 datasheet revision 3.4, the I219 datasheet 612523 and the PCH's volume 2, 631120), which the first round did not have. The transmit cause is per part. The 82574's `TxQ0` "indicates transmit queue 0 write back" and is the name MSI-X mode maps to a vector (§10.2.4.1, §7.4.2), so it keeps both names. The PCH's function has an MSI capability and no MSI-X one (631120 §8.1.15), and no document says what its bit 22 is, so the I219 unmasks `TXDW` alone. The stub now models that part in MSI mode, where it had modelled both by `IVAR`, and delivers a cause already recorded the moment it is unmasked (§7.4.3). No Intel document says what either part does with a descriptor it holds when its link goes away, so the driver stops depending on it: there is no room while the link is down, and a pass that reads `LSC` over unsent descriptors resets the function and brings it up again. §10.2.6.7 lets software write the ring's head only after a reset, so `open`'s bring-up is split out and run a second time, with the multicast table kept. The stub holds frames while its link is down and, by a permit, never sends the ones it held. `Counters` and `inspect` gain a ring found full, a wake armed, a wake taken and frames given back unsent; `Counters::too_long` goes with its last reader. virtio's transmit queue becomes `TxQueue`, which is memory alone, and two host tests drive it over a plain allocation: no room with every head in flight, room back by exactly the heads the device used, and a frame offered to a full queue dying by name. Filed: the two values the bring-up writes against revision 3.4. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...nst-the-82574-datasheets-newer-revision.md | 37 ++ ...cs-room-and-wake-are-unread-on-hardware.md | 26 +- toyos-i219/src/lib.rs | 592 +++++++++++------- toyos-i219/src/regs.rs | 12 +- toyos-i219/src/stub.rs | 95 ++- toyos-i219/src/tests.rs | 324 +++++++--- userland/netstack/src/card.rs | 12 +- userland/netstack/src/i219.rs | 21 +- userland/netstack/src/virtio_net.rs | 196 ++++-- 9 files changed, 919 insertions(+), 396 deletions(-) create mode 100644 issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md diff --git a/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md b/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md new file mode 100644 index 00000000000..abe7976efe0 --- /dev/null +++ b/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md @@ -0,0 +1,37 @@ +--- +status: open +kind: defect +opened: 2026-10-08 +--- + +# The Intel driver writes `TXDCTL` and `TIDV` against the 82574 datasheet's newer revision + +`toyos-i219`'s header cites revision 2.7 of the *Intel 82574 GbE Controller +Family Datasheet*. The copy intel.com serves is revision 3.4 (June 2014), and +it reads differently in two places the driver's bring-up writes: + +- `TXDCTL` bit 22: "Reserved. Must be set to 1b for proper operation." The + revision history gives it to revision 3.1 ("changed bit 22 value to 1b"). + `regs::txdctl::SUGGESTED` is `GRAN` and a `WTHRESH` of 1, written whole, so + the bit is cleared. +- `TIDV`: "Counts in units of 1.024 microseconds. A value of 0 is not allowed." + `open` writes 0, which is also the register's reset value; the same section + says a descriptor with `IDE` clear, which is every one the driver publishes, + is not delayed by it. + +Revision 3.4 also numbers `TXDCTL` §10.2.6.11 where `regs.rs` cites §10.2.6.10; +every other section the tree cites that was compared sits at the same number. + +Neither has been seen to matter: the T14 has sent frames with both. Whether +either register means the same on the PCH's MAC no document says (631120 §8.2 +publishes neither). + +## Owner + +Stage 2 of `issues/the-lan-is-not-yet-production-grade.md`. + +## Exit + +`toyos-i219`'s header names the revision its citations were last read in, the +bring-up writes `TXDCTL` with bit 22 set on the 82574 and leaves `TIDV` alone, +and the stub refuses a `TXDCTL` write that clears the bit. diff --git a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md index de2b08a14f2..020c23d87c8 100644 --- a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md +++ b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md @@ -9,18 +9,25 @@ opened: 2026-10-08 netstack offers its card a frame only when the driver counts room for it, and where there is none it sleeps on the claim until `toyos_i219::I219::wake_on_room`'s cause arrives. On the I219 and the 82574 every reading of that path is the -driver's own model (`toyos-i219/src/stub.rs`), which is the 82574 datasheet +driver's own model (`toyos-i219/src/stub.rs`), which is Intel's documents written down: no image gives netstack `pci:8086:15fc`, and the harness has no Intel card, so neither the T14 nor QEMU's `e1000e` has run it. -What the model cannot say of the T14's part: which of the cause's two names -(`TXDW`, or `TXQ0` through `IVAR`) the PCH's MAC raises a message for, and -whether a ring left full across a link going down is written back when the -link returns. +What the documents do not say of the T14's part, and the model therefore takes +on trust: that the PCH's MAC raises a message for `TXDW` once it is unmasked. +Its function has an MSI capability and no MSI-X one (631120 §8.1.15), so the +82574's §7.4.1 is the mapping the driver assumes; 631120 publishes no +interrupt register of that MAC at all. A wake that never comes is no wedge and no lost frame: room is counted from the descriptors in memory, so the next pass any other event begins finds it. -It is a stall until that pass. +It is a stall until that pass, and it reads as `transmit.wake_armed` ahead of +`transmit.wake_taken`. + +Not in the exit: the reset that takes the transmit ring back when the link +changes over unsent frames. It needs a link to drop under traffic, which no +row can stage. A boot where it happens says so by itself: netstack's line +"the link changed over unsent frames", and `descriptors.unsent` in `inspect`. ## Owner @@ -30,6 +37,7 @@ first boot that gives netstack the wired card. ## Exit A T14 boot whose netstack drives `8086:15fc` sends more than -`toyos_i219::TX_RING - 1` frames in one pass, and its readback shows the pass -after the full ring begun by the card's interrupt and every frame sent -(`descriptors.sent` against `wire.sent` in `inspect`). +`toyos_i219::TX_RING - 1` frames in one pass, and `inspect` then answers +`transmit.full`, `transmit.wake_armed` and `transmit.wake_taken` each at least +1, `descriptors.unsent` 0, and `descriptors.sent` equal to `wire.sent`; the +boot's log carries no "cannot be driven on" line from netstack. diff --git a/toyos-i219/src/lib.rs b/toyos-i219/src/lib.rs index 13ee40efb5a..e68e25c5f8b 100644 --- a/toyos-i219/src/lib.rs +++ b/toyos-i219/src/lib.rs @@ -38,6 +38,15 @@ //! cause that says a descriptor came back. Nothing here drops a frame for want //! of a descriptor, and nothing waits on the part. //! +//! **A link that is down has no room, and a link that changed gives the ring +//! back.** No Intel document says what either part does with a descriptor it +//! holds when its link goes away, so nothing here depends on it: no frame is +//! published while the link is down, and a pass that reads `LSC` with a +//! descriptor still unsent resets the function and brings it up again +//! ([`I219::begin_pass`]) — §10.2.6.7 lets software write the ring's head +//! only "after a reset (hardware reset or CTRL.RST) and before enabling the +//! transmit function", so that reset is the one way a ring is taken back. +//! //! # The device is not trusted //! //! Every number in a written-back descriptor is the device's, and this driver @@ -388,6 +397,32 @@ pub struct Pass { pub causes: u32, /// Whether the link's state is not what it was at the last pass. pub link_changed: bool, + /// Whether this pass reset the function and brought it up again, to take + /// back a transmit ring a link change left with unsent descriptors. + pub rearmed: bool, +} + +/// Why [`I219::begin_pass`] did not begin a pass. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum PassRefused { + /// The claim refused its interrupt record, in its own word: the function + /// is no longer this driver's. + Claim(C), + /// The function did not come back up from the reset that takes its + /// transmit ring back. + Rearm(Refusal), +} + +impl core::fmt::Display for PassRefused { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Claim(why) => write!(f, "its claim refused an interrupt read: {why:?}"), + Self::Rearm(why) => write!( + f, + "it did not come back from the reset that takes its transmit ring back: {why}" + ), + } + } } /// Everything this driver has refused, dropped or been told about, for the one @@ -407,8 +442,17 @@ pub struct Counters { pub overruns: u32, /// `ICR.RXDMT0`: free descriptors fell to the threshold. pub starved: u32, - /// Frames the caller offered that no transmit buffer holds. - pub too_long: u32, + /// Times a caller with a frame found the transmit ring with no room: + /// [`I219::wake_on_room`] answering 0 on a link that is up. + pub tx_full: u32, + /// Times the transmit cause was unmasked for such a caller. + pub tx_wake_armed: u32, + /// Passes that read the transmit cause while it was unmasked: the wake a + /// full ring was waiting on, taken. + pub tx_wake_taken: u32, + /// Frames published and never sent: the descriptors a link change gave + /// back ([`I219::begin_pass`]). + pub unsent: u32, /// Messages that arrived with no cause set in `ICR` — §7.4.5's spurious /// interrupt. It costs a pass and nothing else. pub spurious: u32, @@ -421,11 +465,11 @@ pub struct Counters { impl Counters { /// The counts that are worth a line: every one but [`Self::spurious`], - /// [`Self::sent`] and [`Self::received`], which move on their own on a - /// working card, so a diagnostic keyed on them would print on nothing - /// having gone wrong. + /// [`Self::sent`], [`Self::received`] and the three of a full transmit + /// ring, which move on their own on a working card, so a diagnostic keyed + /// on them would print on nothing having gone wrong. pub fn anomalies(&self) -> Self { - Self { spurious: 0, sent: 0, received: 0, ..*self } + Self { spurious: 0, sent: 0, received: 0, tx_full: 0, tx_wake_armed: 0, tx_wake_taken: 0, ..*self } } } @@ -673,6 +717,29 @@ pub enum Part { I219, } +impl Part { + /// The cause that says a transmit descriptor was written back, as this + /// part raises a message for it. + /// + /// §7.2.8: "Any write backs are performed; either with the RS bit set or + /// when accumulated descriptors are written back [...] Transmit Descriptor + /// Write Back (ICR.TXDW)", and §10.2.4.1 gives the 82574 the same event a + /// second time as `TxQ0`, "Indicates transmit queue 0 write back" — the + /// name §7.4.2 maps to a vector, where "the ICR[24:20] bits reflect + /// specific interrupt causes" in MSI-X mode. Both are unmasked on it. + /// + /// **The PCH's MAC is never in that mode, and gets `TXDW` alone**: its + /// function publishes a Message Signaled Interrupt capability and no MSI-X + /// one (631120 §8.1, §8.1.15), so §7.4.1's fixed mapping is the only one + /// it has, and what its bit 22 means no Intel document publishes. + fn tx_done(self) -> u32 { + match self { + Self::E82574 => cause::TXDW | cause::TXQ0, + Self::I219 => cause::TXDW, + } + } +} + /// What [`I219::open`] found on the way up, for the one line a caller prints /// about a function that raised no link. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -717,7 +784,7 @@ pub struct I219 { tx_next: usize, /// Next transmit descriptor to reclaim. tx_clean: usize, - /// Whether [`cause::TX_DONE`] is unmasked: a caller found the ring full + /// Whether [`Part::tx_done`] is unmasked: a caller found the ring full /// and no pass has begun since. tx_wake: bool, counters: Counters, @@ -733,6 +800,216 @@ impl Drop for I219 { } } +/// What [`program`] leaves beside the registers it wrote. +struct Programmed { + mac: [u8; 6], + brought_up: BringUp, +} + +/// §4.6's bring-up from the reset to the interrupt mask: everything +/// [`I219::open`] programs, which is also everything a reset takes away. +fn program( + part: Part, + regs: &R, + clock: &C, + dma: &D, +) -> Result { + // The PHY is brought within reach before the reset, and the reset + // takes it with the MAC: [`wake`]'s header is why, on the part whose + // PHY is not on the MAC's own die. + let (woke, whole) = match part { + Part::I219 => { + let woke = phy::wake(regs, clock, None); + (Some(woke), Whole::WithThePhy { after: Some(clock.nanos()) }) + } + Part::E82574 => (None, Whole::MacAlone), + }; + let Reset { master_quiet, at: reset_at, full, released } = + reset(regs, clock, whole)?; + + // §10.2.5.23: the reset reloads entry 0 from the NVM, so this is read + // after it and not before. + let low = regs.read(regs::RAL0); + let high = regs.read(regs::RAH0); + if high & rah::AV == 0 { + return Err(Refusal::NoStationAddress); + } + let mac = [ + low as u8, + (low >> 8) as u8, + (low >> 16) as u8, + (low >> 24) as u8, + high as u8, + (high >> 8) as u8, + ]; + + // What the PCH's MAC is given after its reset and before its rings: + // [`pch`]'s header. After the last refusal above, so the one place a + // refusal has the word to give back is below. + if part == Part::I219 { + pch::prepare(regs); + } + + // §4.6.5: "Set up the Multicast Table Array (MTA) per software. This + // generally means zeroing all entries initially." The table this part + // has, and nothing past it. + let table = match part { + Part::E82574 => regs::MTA_DWORDS, + Part::I219 => regs::MTA_DWORDS_PCH, + }; + for entry in 0..table { + regs.write(regs::MTA + entry * 4, 0); + } + + // §4.6.3.1: "Refer to the PHY documentation for the initialization and + // link setup steps. The device driver uses the MDIC register to + // initialize the PHY and setup the link." The PHY documentation this + // driver has is the I219's, and §10.2.2.7 addresses the 82574's own PHY + // under a scheme of its own — so the sequence is refused by name on the + // part it was not written from. + let phy = match part { + Part::I219 => phy::bring_up(regs, clock, reset_at, released), + Part::E82574 => Err(phy::PhyRefusal::NotThisRegisterMap), + }; + + // §10.2.2.1: `SLU` is what lets the MAC see the PHY's link at all; + // `ASDE` must be zero on this family; forcing speed or duplex would + // override what auto-negotiation resolved; and this driver negotiates + // no flow control and strips no VLAN tag. §3.1.3.10's master disable + // goes with them and is not preserved: a part that came out of the + // reset still blocking master requests would fetch no descriptor and + // write back no frame, and nothing else in this bring-up would say so. + let held = regs.read(regs::CTRL); + let wanted = (held + & !(ctrl::GIO_MASTER_DISABLE + | ctrl::PHY_RST + | ctrl::ASDE + | ctrl::ILOS + | ctrl::FRCSPD + | ctrl::FRCDPLX + | ctrl::RFCE + | ctrl::TFCE + | ctrl::VME)) + | ctrl::SLU; + regs.write(regs::CTRL, wanted); + + // §10.2.4.7: "If any bits are set in EIAC, the ICR register should not + // be read" — and this driver reads it, so auto-clear stays off and + // every cause is acknowledged by the write-back in `begin_pass`. + regs.write(regs::EIAC, 0); + + // A refusal from here on gives the word to the firmware back: nothing + // holds the function that a drop would let go. + let armed = rings_and_mask(regs, dma); + if armed.is_err() && part == Part::I219 { + pch::release(regs); + } + armed?; + Ok(Programmed { mac, brought_up: BringUp { master_quiet, woke, reset: full, phy } }) +} + +/// The rest of §4.6: the vector allocation, both rings, the transmitter, the +/// receiver, and the interrupt mask last. +fn rings_and_mask(regs: &R, dma: &D) -> Result<(), Refusal> { + + // §10.2.4.9: `IVAR` allocates every cause to no vector at reset, so a + // part in MSI-X mode with it unprogrammed fills `ICR` and delivers + // nothing. Read back, because the document defines the register only + // "in MSI-X mode" and says nothing about what a part outside that mode + // answers — so a part that does not take the write is refused here + // rather than driven on a guess about which interrupt it would raise. + regs.write(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); + accepted(regs, regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; + + // No moderation on either side: §10.2.4.2's throttle and the two + // receive timers all hold an interrupt back, and what this driver + // waits on is the frame that has already arrived. + regs.write(regs::ITR, 0); + regs.write(regs::RDTR, 0); + regs.write(regs::RADV, 0); + regs.write(regs::TIDV, 0); + regs.write(regs::TADV, 0); + + arm_rx_ring(regs, dma); + arm_tx_ring(regs, dma); + + // §4.6.6, in its order: the write-back policy, the gap, then the + // transmitter. + regs.write(regs::TXDCTL, txdctl::SUGGESTED); + regs.write(regs::TIPG, regs::TIPG_DEFAULT); + regs.write(regs::TCTL, TX_CONTROL); + accepted(regs, regs::TCTL, TX_CONTROL)?; + + // §4.6.5.1: the receiver last, "only after all other setup is + // accomplished". + let rx = rctl::EN | rctl::BAM | rctl::SECRC | rctl::BSIZE_2048; + regs.write(regs::RCTL, rx); + accepted(regs, regs::RCTL, rx)?; + + // §4.6.5: and only now the mask, so no cause can arrive before there + // is a ring to answer it with. + regs.write(regs::IMS, cause::ENABLED | cause::ENABLED_MSIX); + Ok(()) +} + +/// A register wrote what it was told, so a window that is not this register +/// file is refused here instead of looking like a dead network. +fn accepted(regs: &R, reg: usize, wrote: u32) -> Result<(), Refusal> { + let read = regs.read(reg); + if read & wrote == wrote { + Ok(()) + } else { + Err(Refusal::NotAccepted { reg, wrote, read }) + } +} + +/// Publish every receive descriptor and hand the ring to the device. +/// +/// §4.6.5.1: base, length, head, then "the tail pointer should be set to +/// point one descriptor beyond the end" — so the ring holds `RX_RING` +/// buffers and `RX_RING - 1` of them are the device's at any moment, the +/// last being the one §7.1.8's "head equals tail is empty" costs. +fn arm_rx_ring(regs: &R, dma: &D) { + for index in 0..RX_RING { + publish_rx(dma, index); + } + let base = dma.device_addr(OFF_RX_RING); + dma.publish(); + regs.write(regs::RDBAL, base as u32); + regs.write(regs::RDBAH, (base >> 32) as u32); + regs.write(regs::RDLEN, (RX_RING * rx_desc::BYTES) as u32); + regs.write(regs::RDH, 0); + regs.write(regs::RDT, (RX_RING - 1) as u32); +} + +fn arm_tx_ring(regs: &R, dma: &D) { + for index in 0..TX_RING { + let at = OFF_TX_RING + index * tx_desc::BYTES; + dma.write(at, 0); + dma.write(at + 8, 0); + } + let base = dma.device_addr(OFF_TX_RING); + dma.publish(); + regs.write(regs::TDBAL, base as u32); + regs.write(regs::TDBAH, (base >> 32) as u32); + regs.write(regs::TDLEN, (TX_RING * tx_desc::BYTES) as u32); + regs.write(regs::TDH, 0); + regs.write(regs::TDT, 0); +} + +/// Write descriptor `index` back as an empty buffer the device may fill. +/// +/// The status word is zeroed before the address is published, because +/// §7.1.8 says software can "zero the status byte in the descriptor to +/// make it ready for reuse" and a stale `DD` left behind would be read as +/// a completion for a frame that never arrived. +fn publish_rx(dma: &D, index: usize) { + let at = OFF_RX_RING + index * rx_desc::BYTES; + let buffer = OFF_RX_BUFS + index * RX_BUF_BYTES; + dma.write(at + 8, 0); + dma.write(at, dma.device_addr(buffer)); +} + impl I219 { /// Bring the function up in the order §4.6 fixes: reset with every /// interrupt masked, the station address, the multicast table, the PHY, the @@ -751,90 +1028,7 @@ impl I219 { if (dma.bytes() as u64) < GRANT_BYTES { return Err(Refusal::Grant { given: dma.bytes(), needed: GRANT_BYTES as usize }); } - // The PHY is brought within reach before the reset, and the reset - // takes it with the MAC: [`wake`]'s header is why, on the part whose - // PHY is not on the MAC's own die. - let (woke, whole) = match part { - Part::I219 => { - let woke = phy::wake(®s, &clock, None); - (Some(woke), Whole::WithThePhy { after: Some(clock.nanos()) }) - } - Part::E82574 => (None, Whole::MacAlone), - }; - let Reset { master_quiet, at: reset_at, full, released } = - reset(®s, &clock, whole)?; - - // §10.2.5.23: the reset reloads entry 0 from the NVM, so this is read - // after it and not before. - let low = regs.read(regs::RAL0); - let high = regs.read(regs::RAH0); - if high & rah::AV == 0 { - return Err(Refusal::NoStationAddress); - } - let mac = [ - low as u8, - (low >> 8) as u8, - (low >> 16) as u8, - (low >> 24) as u8, - high as u8, - (high >> 8) as u8, - ]; - - // What the PCH's MAC is given after its reset and before its rings: - // [`pch`]'s header. After the last refusal that comes before `Self` - // exists, so every refusal after it is a drop that lets the function go. - if part == Part::I219 { - pch::prepare(®s); - } - - // §4.6.5: "Set up the Multicast Table Array (MTA) per software. This - // generally means zeroing all entries initially." The table this part - // has, and nothing past it. - let table = match part { - Part::E82574 => regs::MTA_DWORDS, - Part::I219 => regs::MTA_DWORDS_PCH, - }; - for entry in 0..table { - regs.write(regs::MTA + entry * 4, 0); - } - - // §4.6.3.1: "Refer to the PHY documentation for the initialization and - // link setup steps. The device driver uses the MDIC register to - // initialize the PHY and setup the link." The PHY documentation this - // driver has is the I219's, and §10.2.2.7 addresses the 82574's own PHY - // under a scheme of its own — so the sequence is refused by name on the - // part it was not written from. - let phy = match part { - Part::I219 => phy::bring_up(®s, &clock, reset_at, released), - Part::E82574 => Err(phy::PhyRefusal::NotThisRegisterMap), - }; - - // §10.2.2.1: `SLU` is what lets the MAC see the PHY's link at all; - // `ASDE` must be zero on this family; forcing speed or duplex would - // override what auto-negotiation resolved; and this driver negotiates - // no flow control and strips no VLAN tag. §3.1.3.10's master disable - // goes with them and is not preserved: a part that came out of the - // reset still blocking master requests would fetch no descriptor and - // write back no frame, and nothing else in this bring-up would say so. - let held = regs.read(regs::CTRL); - let wanted = (held - & !(ctrl::GIO_MASTER_DISABLE - | ctrl::PHY_RST - | ctrl::ASDE - | ctrl::ILOS - | ctrl::FRCSPD - | ctrl::FRCDPLX - | ctrl::RFCE - | ctrl::TFCE - | ctrl::VME)) - | ctrl::SLU; - regs.write(regs::CTRL, wanted); - - // §10.2.4.7: "If any bits are set in EIAC, the ICR register should not - // be read" — and this driver reads it, so auto-clear stays off and - // every cause is acknowledged by the write-back in `begin_pass`. - regs.write(regs::EIAC, 0); - + let Programmed { mac, brought_up } = program(part, ®s, &clock, &dma)?; let mut nic = Self { part, regs, @@ -842,7 +1036,7 @@ impl I219 { dma, irq, mac, - brought_up: BringUp { master_quiet, woke, reset: full, phy }, + brought_up, link: Link::default(), opened_at: 0, link_up_at: None, @@ -855,50 +1049,12 @@ impl I219 { counters: Counters::default(), wire: Wire::default(), }; - - // §10.2.4.9: `IVAR` allocates every cause to no vector at reset, so a - // part in MSI-X mode with it unprogrammed fills `ICR` and delivers - // nothing. Read back, because the document defines the register only - // "in MSI-X mode" and says nothing about what a part outside that mode - // answers — so a part that does not take the write is refused here - // rather than driven on a guess about which interrupt it would raise. - nic.regs.write(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); - nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; - - // No moderation on either side: §10.2.4.2's throttle and the two - // receive timers all hold an interrupt back, and what this driver - // waits on is the frame that has already arrived. - nic.regs.write(regs::ITR, 0); - nic.regs.write(regs::RDTR, 0); - nic.regs.write(regs::RADV, 0); - nic.regs.write(regs::TIDV, 0); - nic.regs.write(regs::TADV, 0); - - nic.arm_rx_ring(); - nic.arm_tx_ring(); - - // §4.6.6, in its order: the write-back policy, the gap, then the - // transmitter. - nic.regs.write(regs::TXDCTL, txdctl::SUGGESTED); - nic.regs.write(regs::TIPG, regs::TIPG_DEFAULT); - nic.regs.write(regs::TCTL, TX_CONTROL); - nic.accepted(regs::TCTL, TX_CONTROL)?; - - // §4.6.5.1: the receiver last, "only after all other setup is - // accomplished". - let rx = rctl::EN | rctl::BAM | rctl::SECRC | rctl::BSIZE_2048; - nic.regs.write(regs::RCTL, rx); - nic.accepted(regs::RCTL, rx)?; - - // §4.6.5: and only now the mask, so no cause can arrive before there - // is a ring to answer it with. - nic.regs.write(regs::IMS, cause::ENABLED | cause::ENABLED_MSIX); - nic.opened_at = nic.clock.nanos(); nic.refresh_link(); Ok(nic) } + /// Pass frames sent to the multicast address `group`: the one bit of the /// Multicast Table Array its hash names on this part ([`regs::mta_bit_82574`], /// [`regs::mta_bit_pch`]) is set, and @@ -912,64 +1068,6 @@ impl I219 { self.regs.write(at, self.regs.read(at) | 1 << bit); } - /// A register wrote what it was told, so a window that is not this register - /// file is refused here instead of looking like a dead network. - fn accepted(&self, reg: usize, wrote: u32) -> Result<(), Refusal> { - let read = self.regs.read(reg); - if read & wrote == wrote { - Ok(()) - } else { - Err(Refusal::NotAccepted { reg, wrote, read }) - } - } - - /// Publish every receive descriptor and hand the ring to the device. - /// - /// §4.6.5.1: base, length, head, then "the tail pointer should be set to - /// point one descriptor beyond the end" — so the ring holds `RX_RING` - /// buffers and `RX_RING - 1` of them are the device's at any moment, the - /// last being the one §7.1.8's "head equals tail is empty" costs. - fn arm_rx_ring(&mut self) { - for index in 0..RX_RING { - self.publish_rx(index); - } - let base = self.dma.device_addr(OFF_RX_RING); - self.dma.publish(); - self.regs.write(regs::RDBAL, base as u32); - self.regs.write(regs::RDBAH, (base >> 32) as u32); - self.regs.write(regs::RDLEN, (RX_RING * rx_desc::BYTES) as u32); - self.regs.write(regs::RDH, 0); - self.regs.write(regs::RDT, self.rx_tail as u32); - } - - fn arm_tx_ring(&mut self) { - for index in 0..TX_RING { - let at = OFF_TX_RING + index * tx_desc::BYTES; - self.dma.write(at, 0); - self.dma.write(at + 8, 0); - } - let base = self.dma.device_addr(OFF_TX_RING); - self.dma.publish(); - self.regs.write(regs::TDBAL, base as u32); - self.regs.write(regs::TDBAH, (base >> 32) as u32); - self.regs.write(regs::TDLEN, (TX_RING * tx_desc::BYTES) as u32); - self.regs.write(regs::TDH, 0); - self.regs.write(regs::TDT, 0); - } - - /// Write descriptor `index` back as an empty buffer the device may fill. - /// - /// The status word is zeroed before the address is published, because - /// §7.1.8 says software can "zero the status byte in the descriptor to - /// make it ready for reuse" and a stale `DD` left behind would be read as - /// a completion for a frame that never arrived. - fn publish_rx(&mut self, index: usize) { - let at = OFF_RX_RING + index * rx_desc::BYTES; - let buffer = OFF_RX_BUFS + index * RX_BUF_BYTES; - self.dma.write(at + 8, 0); - self.dma.write(at, self.dma.device_addr(buffer)); - } - pub fn mac(&self) -> [u8; 6] { self.mac } @@ -1026,21 +1124,27 @@ impl I219 { /// refuses to answer at all is not a pass with no messages in it — the /// function is no longer this driver's, and the refusal is handed up rather /// than counted as quiet. - pub fn begin_pass(&mut self) -> Result { + /// + /// **A pass that reads `LSC` with a transmit descriptor still unsent takes + /// the ring back**, by the reset and the bring-up [`Self::open`] runs: + /// the frames in it are counted [`Counters::unsent`], a frame received and + /// not yet handed up goes with them, and the pass lasts as long as a + /// bring-up does. Every [`Frame`] is given back before a pass begins. + pub fn begin_pass(&mut self) -> Result> { self.rx_budget = RX_BUDGET; let messages = match self.irq.taken() { Ok(count) => count, Err(why) if why == I::IDLE => 0, - Err(why) => return Err(why), + Err(why) => return Err(PassRefused::Claim(why)), }; // §10.2.4.6: the transmit cause is masked again, because it was wanted // for one wake and left unmasked it is a message for every frame sent. // Before the room this pass's sends ask for: a ring still full arms it // again in [`Self::wake_on_room`]. - if self.tx_wake { - self.regs.write(regs::IMC, cause::TX_DONE); - self.tx_wake = false; + let waited = core::mem::take(&mut self.tx_wake); + if waited { + self.regs.write(regs::IMC, self.part.tx_done()); } // Once, and written back. §10.2.4.1's case 3 says a read with no @@ -1064,6 +1168,9 @@ impl I219 { if messages > 0 && acknowledged == 0 { self.counters.spurious = self.counters.spurious.saturating_add(messages); } + if waited && causes & self.part.tx_done() != 0 { + self.counters.tx_wake_taken = self.counters.tx_wake_taken.saturating_add(1); + } // On `LSC` and on every pass that found no cause at all: the link can // also come up before the mask was written, and then no `LSC` is ever @@ -1072,7 +1179,52 @@ impl I219 { if causes & cause::LSC != 0 || !self.link.is_up() { self.refresh_link(); } - Ok(Pass { messages, causes, link_changed: self.link != before }) + let rearmed = causes & cause::LSC != 0 && self.unsent() > 0; + if rearmed { + self.rearm().map_err(PassRefused::Rearm)?; + } + Ok(Pass { messages, causes, link_changed: self.link != before, rearmed }) + } + + /// Transmit descriptors published and not written back, the written-back + /// ones reclaimed first. + fn unsent(&mut self) -> usize { + self.reclaim_tx(); + (self.tx_next + TX_RING - self.tx_clean) % TX_RING + } + + /// Reset the function and bring it up again, as [`Self::open`] did, with + /// both rings empty and the multicast table as it stood. + /// + /// §10.2.6.1 on `TCTL.EN`: "Software should combine this with a reset if + /// the packets in the FIFO need to be flushed" — and §10.2.6.7's head is + /// software's to write only after one. + fn rearm(&mut self) -> Result<(), Refusal> { + let unsent = self.unsent() as u32; + let dwords = match self.part { + Part::E82574 => regs::MTA_DWORDS, + Part::I219 => regs::MTA_DWORDS_PCH, + }; + let mut table = [0u32; regs::MTA_DWORDS]; + for (entry, word) in table[..dwords].iter_mut().enumerate() { + *word = self.regs.read(regs::MTA + entry * 4); + } + let Programmed { mac: _, brought_up } = program(self.part, &self.regs, &self.clock, &self.dma)?; + for (entry, word) in table[..dwords].iter().enumerate() { + self.regs.write(regs::MTA + entry * 4, *word); + } + self.brought_up = brought_up; + self.rx_next = 0; + self.rx_tail = RX_RING - 1; + self.tx_next = 0; + self.tx_clean = 0; + self.counters.unsent = self.counters.unsent.saturating_add(unsent); + // The link change the bring-up itself made is acknowledged before the + // link is read: left in `ICR` it would be the next pass's reason to + // take back a ring that has frames in it again. + self.regs.write(regs::ICR, cause::LSC); + self.refresh_link(); + Ok(()) } /// Re-read `STATUS` and take what it says about the link. @@ -1157,12 +1309,12 @@ impl I219 { /// one, walk `RDT` backwards over the rest of the ring. /// /// **Ready is read out of the descriptor, so there is no second record of - /// it to disagree.** [`Self::publish_rx`] zeroes the status word, and a + /// it to disagree.** [`publish_rx`] zeroes the status word, and a /// descriptor still in a caller's hands holds the non-zero word the device /// wrote back — `DD` is what [`Self::poll_rx`] took it on. The device /// touches neither until the tail passes it. fn give_back(&mut self, index: usize) { - self.publish_rx(index); + publish_rx(&self.dma, index); let mut tail = self.rx_tail; loop { let next = (tail + 1) % RX_RING; @@ -1187,31 +1339,50 @@ impl I219 { } /// How many frames the transmit ring takes now, every descriptor the part - /// has written back taken first. + /// has written back taken first — and none while the link is down, whose + /// return is `LSC`'s to say. /// /// §7.2.4: hardware owns `[TDH..TDT)`, so a ring filled to the last /// descriptor would wrap the tail onto the head and read as empty — one /// descriptor is never handed out, and a full ring is [`TX_RING`]` - 1` in /// flight. pub fn tx_room(&mut self) -> usize { - self.reclaim_tx(); - (self.tx_clean + TX_RING - 1 - self.tx_next) % TX_RING + if !self.link.is_up() { + return 0; + } + TX_RING - 1 - self.unsent() } /// Ask for a message when the part next writes a transmit descriptor /// back, and answer the room there is now that it has been asked. /// - /// §10.2.4.5: a write of [`cause::TX_DONE`] to `IMS` unmasks it, and it - /// stays unmasked until the next [`Self::begin_pass`]. + /// §10.2.4.5: "A particular interrupt can be enabled by writing a 1b to + /// the corresponding mask bit", and [`Part::tx_done`] stays unmasked until + /// the next [`Self::begin_pass`]. Each frame sent is a write-back and each + /// write-back the cause: `TXDCTL`'s description has "all descriptors + /// written back" with `GRAN` set, §7.2.4.2 writes them back "only when + /// TXDCTL.WTHRESH number of descriptors are ready", which at one is each, + /// and §7.2.8 sets `TXDW` when "any write backs are performed". /// /// **Unmasked first and counted after**, so no write-back is lost between - /// the two: one that landed before the mask was written raised nothing and - /// is in the count, and one that lands after it is a message. A caller - /// answered 0 waits on its claim. + /// the two: one that landed before the mask was written is in the count, + /// and one that lands after it is a message. §7.4.3 signals an interrupt + /// "when unmasked bits in this register are set", so the first may be a + /// message as well, which costs a pass. A caller answered 0 waits on its + /// claim. + /// + /// With the link down nothing is unmasked and the answer is 0: the wake + /// is the link's own cause, which is never masked. pub fn wake_on_room(&mut self) -> usize { + let room = self.tx_room(); + if room > 0 || !self.link.is_up() { + return room; + } + self.counters.tx_full = self.counters.tx_full.saturating_add(1); if !self.tx_wake { - self.regs.write(regs::IMS, cause::TX_DONE); + self.regs.write(regs::IMS, self.part.tx_done()); self.tx_wake = true; + self.counters.tx_wake_armed = self.counters.tx_wake_armed.saturating_add(1); } self.tx_room() } @@ -1224,13 +1395,8 @@ impl I219 { pub fn tx_reserve(&mut self, len: usize) -> Option { // §7.2.10.1: one legacy descriptor carries one buffer, and this // driver's is `TX_BUF_BYTES`. A longer frame is refused rather than - // truncated into one, and counted because it is a caller that offered - // more than it was told it could. - if len > TX_BUF_BYTES { - self.counters.too_long = self.counters.too_long.saturating_add(1); - return None; - } - if self.tx_room() == 0 { + // truncated into one. + if len > TX_BUF_BYTES || self.tx_room() == 0 { return None; } let index = self.tx_next; diff --git a/toyos-i219/src/regs.rs b/toyos-i219/src/regs.rs index 8e590e0a942..5219d230fdf 100644 --- a/toyos-i219/src/regs.rs +++ b/toyos-i219/src/regs.rs @@ -358,7 +358,8 @@ pub mod extcnf { /// Interrupt cause bits, shared by `ICR`, `ICS`, `IMS` and `IMC` (§10.2.4.1). pub mod cause { - /// Transmit Descriptor Written Back (bit 0). + /// Transmit Descriptor Written Back (bit 0). §10.2.4.1: "Set when hardware + /// processes a descriptor with RS set." pub const TXDW: u32 = 1 << 0; /// Link Status Change (bit 2) — "set whenever the link status changes /// (either from up to down, or from down to up)". @@ -381,15 +382,6 @@ pub mod cause { /// when every cause has. pub const INT_ASSERTED: u32 = 1 << 31; - /// A transmit descriptor written back, under both of §10.2.4.1's names for - /// it: `TXDW`, and the `TXQ0` §10.2.4.9's `IVAR` gives a vector, which is - /// the only one of the two a part in MSI-X mode raises anything for. - /// §7.2.4.2: the write-back is of a descriptor that carried `RS`, which - /// every one this driver publishes does, and with `IDE` clear no timer - /// holds the cause back. **Not in [`ENABLED`]**: it is unmasked while a - /// caller waits on a full transmit ring, and never otherwise. - pub const TX_DONE: u32 = TXDW | TXQ0; - /// What §4.6.5 tells a driver to unmask: "Suggested bits include RXT, RXO, /// RXDMT and LSC. There is no reason to enable the transmit interrupts." pub const ENABLED: u32 = RXT0 | RXO | RXDMT0 | LSC; diff --git a/toyos-i219/src/stub.rs b/toyos-i219/src/stub.rs index fbbb3278fc1..261ce135028 100644 --- a/toyos-i219/src/stub.rs +++ b/toyos-i219/src/stub.rs @@ -95,6 +95,10 @@ pub struct Permits { /// §7.4.5: an interrupt whose cause was already cleared. "This results in /// a spurious interrupt." pub spurious_interrupts: bool, + /// No Intel document says what a part does with the transmit descriptors + /// it holds when its link goes away. On: it never sends them, and takes + /// no other, until it is reset. Off: it sends them when the link returns. + pub link_loss_strands_the_ring: bool, /// §10.2.4.1 case 3: "Interrupt was not asserted (ICR.INT_ASSERT=0): Read /// has no side affect." The document's own §7.4.5 says instead that "all /// bits in the ICR register are cleared on a read to ICR"; both readings @@ -204,6 +208,7 @@ impl Default for Permits { shadow_head: true, null_padding: true, spurious_interrupts: true, + link_loss_strands_the_ring: true, icr_read_has_no_side_effect: true, master_takes_time_to_quiesce: true, firmware_takes_the_mdio_interface: true, @@ -299,6 +304,11 @@ const OEM_SPEED_BITS: u16 = oem_bits::LOW_POWER_LINK_UP | oem_bits::GIGABIT_DISA /// How many `STATUS` reads §3.1.3.10's master enable stays set for. const MASTER_QUIESCE_READS: u32 = 3; +/// §10.2.4.1's causes below the five §7.4.2 keeps for MSI-X mode: `ICR[19:0]`, +/// which §7.4.1's fixed mapping makes the whole of what a part outside that +/// mode signals. +const CLASSIC_CAUSES: u32 = (1 << 20) - 1; + /// How many reads of `EXTCNF_CTRL` §4.5.2's manageability agent holds the /// interface across before the software request registered under it is granted. const MDIO_FIRMWARE_REQUESTS: u32 = 2; @@ -591,6 +601,9 @@ struct Model { rx_holding: Vec<(usize, u64, Vec)>, tx_head: usize, tx_holding: Vec, + /// The link went away over published descriptors, and this part takes + /// nothing off its transmit ring again before a reset. + tx_stranded: bool, /// Messages the function has sent that the claim has not read. messages: u32, /// One offset in the window that nothing decodes, which answers ones. @@ -697,6 +710,7 @@ impl Model { rx_holding: Vec::new(), tx_head: 0, tx_holding: Vec::new(), + tx_stranded: false, messages: 0, not_decoding: None, written: BTreeSet::new(), @@ -756,6 +770,7 @@ impl Model { self.master_reads = 0; self.rx_holding.clear(); self.tx_holding.clear(); + self.tx_stranded = false; // §4.5.2: the software and hardware ownership bits "are cleared on // reset", and the part takes the interface again "while loading the // extended configuration area" — which is what a reset makes it do. @@ -904,6 +919,7 @@ impl Model { } self.phy.up = up; self.refresh_status(); + self.strand_if_the_link_is_gone(); self.raise(cause::LSC); } @@ -1083,6 +1099,7 @@ impl Model { match reg { regs::CTRL => { let was = self.get(regs::CTRL); + let link_was = self.get(regs::STATUS) & status::LU; assert_eq!( value & CTRL_RESERVED_SET, CTRL_RESERVED_SET, @@ -1149,11 +1166,26 @@ impl Model { self.reset_reads = RESET_READS; } self.refresh_status(); + // §10.2.4.1: `LSC` "is set whenever the link status changes", + // and §4.6.3.2 has `LU` reflect the link "qualified with + // CTRL.SLU" — so a driver that writes `SLU` over a link the + // PHY already has, or resets it away, made a change itself. + if self.get(regs::STATUS) & status::LU != link_was { + self.raise(cause::LSC); + } } - // §10.2.4.5: set, not assign. + // §10.2.4.5: set, not assign — and "a PCIe interrupt is generated + // whenever one of the bits in this register is set, and the + // corresponding interrupt condition occurs", which §7.4.3 reads as + // level: "an interrupt is signaled when unmasked bits in this + // register are set". A cause already recorded is a message the + // moment it is unmasked. regs::IMS => { let held = self.get(regs::IMS); self.set(regs::IMS, held | value); + if self.get(regs::ICR) & value & !held & !cause::INT_ASSERTED != 0 { + self.raise(0); + } } // §10.2.4.6: clear. regs::IMC => { @@ -1733,29 +1765,39 @@ impl Model { } } - /// Record a cause and, if it is unmasked *and* has a vector, send a - /// message. + /// Record a cause and, if it is unmasked and this part's interrupt mode + /// carries it, send a message. /// - /// §10.2.4.1 gives every event two names: the classic cause and the - /// queue-or-other cause §10.2.4.9 allocates a vector to. Both are set in - /// `ICR`; only the second can reach a vector, and only while `IVAR`'s - /// enable bit for it is set. **A driver that programmed no `IVAR` is - /// therefore told nothing at all**, which is the part behaving as specified - /// and not a model being unkind. + /// **The 82574 is in MSI-X mode.** §10.2.4.1 gives every event two names: + /// the classic cause and the queue-or-other cause §10.2.4.9 allocates a + /// vector to. Both are set in `ICR`; only the second can reach a vector, + /// and only while `IVAR`'s enable bit for it is set. A driver that + /// programmed no `IVAR` is therefore told nothing at all, which is the + /// part behaving as specified and not a model being unkind. + /// + /// **The PCH's MAC is in MSI mode**, the only one its function has + /// (631120 §8.1.15, and no MSI-X capability in §8.1's table): §7.4.1's + /// fixed mapping, where every unmasked classic cause is the message, and + /// no second name is set because no document gives its bits 24:20 one. fn raise(&mut self, causes: u32) { let held = self.get(regs::ICR); let mut now = held | causes; - if causes & (cause::RXT0 | cause::RXDMT0) != 0 { - now |= cause::RXQ0; - } - if causes & cause::TXDW != 0 { - now |= cause::TXQ0; - } - if causes & (cause::LSC | cause::RXO) != 0 { - now |= cause::OTHER; + if self.part == Part::E82574 { + if causes & (cause::RXT0 | cause::RXDMT0) != 0 { + now |= cause::RXQ0; + } + if causes & cause::TXDW != 0 { + now |= cause::TXQ0; + } + if causes & (cause::LSC | cause::RXO) != 0 { + now |= cause::OTHER; + } } let unmasked = now & self.get(regs::IMS) & !cause::INT_ASSERTED; - let delivered = unmasked & self.vectored(); + let delivered = match self.part { + Part::E82574 => unmasked & self.vectored(), + Part::I219 => unmasked & CLASSIC_CAUSES, + }; self.set(regs::ICR, if delivered != 0 { now | cause::INT_ASSERTED } else { now }); if delivered != 0 { self.messages = self.messages.saturating_add(1); @@ -1940,6 +1982,17 @@ impl Model { self.raise(cause::RXT0); } + /// [`Permits::link_loss_strands_the_ring`], at every moment the model + /// looks: a link that is down over a published descriptor. + fn strand_if_the_link_is_gone(&mut self) { + let count = self.get(regs::TDLEN) as usize / tx_desc::BYTES; + if count == 0 || self.get(regs::STATUS) & status::LU != 0 { + return; + } + let published = self.tx_head != self.get(regs::TDT) as usize % count; + self.tx_stranded |= self.permits.link_loss_strands_the_ring && published; + } + /// Take every published transmit descriptor and put its frame on the wire. fn transmit(&mut self) { if self.get(regs::TCTL) & tctl::EN == 0 { @@ -1951,6 +2004,11 @@ impl Model { return; } let tail = self.get(regs::TDT) as usize % count; + // A frame leaves on a link. With none the descriptors stay the part's. + self.strand_if_the_link_is_gone(); + if self.tx_stranded || self.get(regs::STATUS) & status::LU == 0 { + return; + } // §7.2.4.1: "The 82574 NEVER fetches descriptors beyond the descriptor // tail pointer." while self.tx_head != tail { @@ -2300,6 +2358,7 @@ impl Nic { Part::I219 => model.refresh_phy_link(), Part::E82574 => { model.refresh_status(); + model.strand_if_the_link_is_gone(); model.raise(cause::LSC); } } diff --git a/toyos-i219/src/tests.rs b/toyos-i219/src/tests.rs index 2771fad2e38..b19e27ebd57 100644 --- a/toyos-i219/src/tests.rs +++ b/toyos-i219/src/tests.rs @@ -596,6 +596,7 @@ fn a_frame_goes_out_whole() { let nic = Nic::new(10); let mut driver = open(&nic); nic.set_link(true); + one_pass(&mut driver); let payload = frame(0x33, 600); let slot = driver.tx_reserve(payload.len()).expect("a free transmit descriptor"); nic.put_bytes(slot.at, &payload); @@ -604,6 +605,32 @@ fn a_frame_goes_out_whole() { assert_eq!(nic.sent(), vec![payload], "{}", nic.because("the frame did not reach the wire")); } +/// Both parts, each with the cause its transmit wake is unmasked under: the +/// 82574 in MSI-X mode under both of §10.2.4.1's names, the PCH's MAC under +/// the classic one alone. +const PARTS: [(Part, u32); 2] = + [(Part::E82574, cause::TXDW | cause::TXQ0), (Part::I219, cause::TXDW)]; + +/// Let a partner onto the wire and take the pass that sees the link. +fn link_up(nic: &Nic, driver: &mut Driver) { + nic.set_link(true); + if nic.part() == Part::I219 { + nic.negotiation_settles(); + } + one_pass(driver); + assert!(driver.link().is_up(), "{}", nic.because("a partner on the wire raised no link")); +} + +/// A part of `part`, opened, its link up and its device held: nothing is +/// sent, written back or reclaimed until `run`. +fn held_with_a_link(seed: u64, part: Part, permits: Permits) -> (Nic, Driver) { + let nic = Nic::with(seed, part, permits); + let mut driver = open(&nic); + link_up(&nic, &mut driver); + nic.hold(); + (nic, driver) +} + /// Publish frames tagged from `first` until the driver refuses one, and answer /// them. The device is held, so nothing is written back while this runs. fn fill(nic: &Nic, driver: &mut Driver, first: u8) -> Vec> { @@ -631,24 +658,28 @@ fn settle(nic: &Nic, driver: &mut Driver) { panic!("{}", nic.because("the transmit ring never emptied")); } +/// The three counts of a full transmit ring, as `inspect` carries them. +fn waits(driver: &Driver) -> (u32, u32, u32) { + let counters = driver.counters(); + (counters.tx_full, counters.tx_wake_armed, counters.tx_wake_taken) +} + /// §7.2.4: hardware owns `[TDH..TDT)`, so filling the last descriptor would /// wrap the tail onto the head and the part would read a full ring as an empty /// one. The ring takes one frame fewer than it has descriptors, says so before /// a frame is offered, and refuses the next. #[test] fn a_full_transmit_ring_answers_room_0() { - let nic = Nic::new(11); - let mut driver = open(&nic); - nic.set_link(true); - // Nothing is sent, written back or reclaimed until `run`: the ring fills. - nic.hold(); - assert_eq!(driver.tx_room(), TX_RING - 1, "{}", nic.because("an empty ring is not the ring less one")); - let out = fill(&nic, &mut driver, 1); - assert_eq!(out.len(), TX_RING - 1, "{}", nic.because("the usable depth is not the ring less one")); - assert_eq!(driver.tx_room(), 0, "{}", nic.because("a full ring said it had room")); - assert!(driver.tx_reserve(64).is_none()); - nic.run(); - assert_eq!(nic.sent(), out, "{}", nic.because("the part did not read the full ring as full")); + for (part, _) in PARTS { + let (nic, mut driver) = held_with_a_link(11, part, Permits::default()); + assert_eq!(driver.tx_room(), TX_RING - 1, "{}", nic.because("an empty ring is not the ring less one")); + let out = fill(&nic, &mut driver, 1); + assert_eq!(out.len(), TX_RING - 1, "{}", nic.because("the usable depth is not the ring less one")); + assert_eq!(driver.tx_room(), 0, "{}", nic.because("a full ring said it had room")); + assert!(driver.tx_reserve(64).is_none()); + nic.run(); + assert_eq!(nic.sent(), out, "{}", nic.because("the part did not read the full ring as full")); + } } /// §7.2.4.2: a descriptor that carried `RS` is written back with `DD`, and @@ -656,80 +687,70 @@ fn a_full_transmit_ring_answers_room_0() { /// with the write-back, by as many frames as were written back. #[test] fn a_written_back_descriptor_returns_room() { - let permits = Permits { batched_writeback: false, ..Permits::default() }; - let nic = Nic::with(23, Part::E82574, permits); - let mut driver = open(&nic); - nic.set_link(true); - nic.hold(); - fill(&nic, &mut driver, 1); - assert_eq!(driver.tx_room(), 0); - nic.run(); - assert_eq!( - driver.tx_room(), - TX_RING - 1, - "{}", - nic.because("every descriptor was written back and the room did not return") - ); - assert_eq!(driver.counters().sent, (TX_RING - 1) as u32); + for (part, _) in PARTS { + let permits = Permits { batched_writeback: false, ..Permits::default() }; + let (nic, mut driver) = held_with_a_link(23, part, permits); + fill(&nic, &mut driver, 1); + assert_eq!(driver.tx_room(), 0); + nic.run(); + assert_eq!( + driver.tx_room(), + TX_RING - 1, + "{}", + nic.because("every descriptor was written back and the room did not return") + ); + assert_eq!(driver.counters().sent, (TX_RING - 1) as u32); + } } /// §4.6.5: "There is no reason to enable the transmit interrupts" — until a /// caller has a frame and no descriptor. The cause is unmasked by the caller -/// that found the ring full, raises the one message that ends its wait, and is -/// masked again by the pass that takes it: a part left with it unmasked sends a -/// message for every frame. +/// that found the ring full, under the names this part raises a message for +/// and no other bit; it raises the one message that ends the wait, and is +/// masked again by the pass that takes it: a part left with it unmasked sends +/// a message for every frame. Each step is counted once. #[test] fn the_transmit_cause_is_armed_only_when_asked() { - let permits = Permits { spurious_interrupts: false, ..Permits::default() }; - let nic = Nic::with(24, Part::E82574, permits); - let mut driver = open(&nic); - nic.set_link(true); - one_pass(&mut driver); - nic.hold(); - - // Not asked: the ring fills and empties, and nothing is said about it. - fill(&nic, &mut driver, 1); - nic.run(); - assert_eq!(nic.peek(regs::IMS) & cause::TX_DONE, 0); - assert_eq!( - nic.pending(), - 0, - "{}", - nic.because("a write-back nobody waited on sent a message") - ); - settle(&nic, &mut driver); - one_pass(&mut driver); - - // Asked, on a full ring. - fill(&nic, &mut driver, 0x40); - assert_eq!(driver.wake_on_room(), 0); - assert_eq!( - nic.peek(regs::IMS), - cause::ENABLED | cause::ENABLED_MSIX | cause::TX_DONE, - "{}", - nic.because("the transmit cause was not unmasked under both its names") - ); - assert_eq!(nic.pending(), 0); - nic.run(); - assert!(nic.pending() > 0, "{}", nic.because("a write-back the caller waited on sent no message")); - let pass = one_pass(&mut driver); - assert_eq!(pass.causes & cause::TX_DONE, cause::TX_DONE); - assert!(driver.tx_room() > 0, "{}", nic.because("the message came and the room did not")); + for (part, tx_done) in PARTS { + let permits = Permits { spurious_interrupts: false, ..Permits::default() }; + let (nic, mut driver) = held_with_a_link(24, part, permits); + let unasked = nic.peek(regs::IMS); + assert_eq!(unasked & (cause::TXDW | cause::TXQ0), 0); + + // Not asked: the ring fills and empties, and nothing is said about it. + fill(&nic, &mut driver, 1); + nic.run(); + assert_eq!(nic.peek(regs::IMS), unasked); + assert_eq!(nic.pending(), 0, "{}", nic.because("a write-back nobody waited on sent a message")); + settle(&nic, &mut driver); + one_pass(&mut driver); + assert_eq!(waits(&driver), (0, 0, 0), "{}", nic.because("a ring nobody waited on was counted")); - // And not asked again: the pass that took the message masked the cause. - assert_eq!( - nic.peek(regs::IMS), - cause::ENABLED | cause::ENABLED_MSIX, - "{}", - nic.because("the pass left the transmit cause unmasked") - ); - settle(&nic, &mut driver); - assert_eq!( - nic.pending(), - 0, - "{}", - nic.because("a write-back after the wake was taken sent a message") - ); + // Asked, on a full ring. + fill(&nic, &mut driver, 0x40); + assert_eq!(driver.wake_on_room(), 0); + assert_eq!( + nic.peek(regs::IMS), + unasked | tx_done, + "{}", + nic.because("the transmit cause was not unmasked under this part's names and no other") + ); + assert_eq!(nic.pending(), 0); + assert_eq!(waits(&driver), (1, 1, 0)); + nic.run(); + assert!(nic.pending() > 0, "{}", nic.because("a write-back the caller waited on sent no message")); + let pass = one_pass(&mut driver); + assert_eq!(pass.causes & tx_done, tx_done); + assert!(driver.tx_room() > 0, "{}", nic.because("the message came and the room did not")); + assert_eq!(waits(&driver), (1, 1, 1), "{}", nic.because("the wake was taken and not counted")); + + // And not asked again: the pass that took the message masked the cause. + assert_eq!(nic.peek(regs::IMS), unasked, "{}", nic.because("the pass left the transmit cause unmasked")); + settle(&nic, &mut driver); + assert_eq!(nic.pending(), 0, "{}", nic.because("a write-back after the wake was taken sent a message")); + one_pass(&mut driver); + assert_eq!(waits(&driver), (1, 1, 1), "{}", nic.because("a cause nobody waited on was counted a wake")); + } } /// A burst several rings long, sent by a caller that asks for room before each @@ -737,12 +758,8 @@ fn the_transmit_cause_is_armed_only_when_asked() { /// order, and every sleep is ended by a message. #[test] fn a_burst_past_the_ring_leaves_whole_on_the_room_it_is_told() { - for seed in 1..=32u64 { - let nic = Nic::new(seed); - let mut driver = open(&nic); - nic.set_link(true); - one_pass(&mut driver); - nic.hold(); + for (seed, (part, _)) in (1..=32u64).flat_map(|seed| PARTS.map(|part| (seed, part))) { + let (nic, mut driver) = held_with_a_link(seed, part, Permits::default()); let mut out: Vec> = Vec::new(); let mut on_the_wire: Vec> = Vec::new(); let mut sleeps = 0; @@ -772,9 +789,118 @@ fn a_burst_past_the_ring_leaves_whole_on_the_room_it_is_told() { } assert!(sleeps > 0, "{}", nic.because("the burst never filled the ring")); assert_eq!(on_the_wire, out, "{}", nic.because("what reached the wire is not the burst")); + let counters = driver.counters(); + assert!(counters.tx_wake_taken > 0, "{}", nic.because("no sleep was ended by the transmit cause")); + assert_eq!(counters.unsent, 0); + } +} + +/// No frame is published on a link that is down: there is no room, nothing is +/// unmasked for it and nothing counted, because the wake is the link's own +/// cause. +#[test] +fn a_link_that_is_down_has_no_room() { + for (part, _) in PARTS { + let nic = Nic::with(25, part, Permits::default()); + let mut driver = open(&nic); + let unasked = nic.peek(regs::IMS); + assert_eq!(driver.tx_room(), 0, "{}", nic.because("a link that is down had room")); + assert!(driver.tx_reserve(64).is_none()); + assert_eq!(driver.wake_on_room(), 0); + assert_eq!(nic.peek(regs::IMS), unasked, "{}", nic.because("a cause was unmasked for a link that is down")); + assert_eq!(waits(&driver), (0, 0, 0)); + link_up(&nic, &mut driver); + assert_eq!(driver.tx_room(), TX_RING - 1); } } +/// What a part does with the descriptors it holds when its link goes away is +/// in no Intel document, so the driver asks nothing of it: the pass that reads +/// `LSC` over unsent descriptors resets the function and brings it up again +/// (§10.2.6.7: the head is software's to write only after a reset). The ring +/// is whole when the link returns, on a part that would never have sent those +/// frames and on one that would; the frames are counted; the multicast table +/// and the receive ring are as they were. +#[test] +fn a_ring_the_link_left_full_is_taken_back() { + const MDNS: [u8; 6] = [0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb]; + let mut reached_at_once = 0; + for ((part, _), strands, seen_down) in PARTS + .into_iter() + .flat_map(|part| [(part, true, true), (part, false, true), (part, true, false), (part, false, false)]) + { + let permits = Permits { link_loss_strands_the_ring: strands, ..Permits::default() }; + let (nic, mut driver) = held_with_a_link(26, part, permits); + let why = |what: &str| nic.because(&format!("{part:?}, strands {strands}, seen down {seen_down}: {what}")); + driver.accept_multicast(MDNS); + let table: Vec = (0..regs::MTA_DWORDS).map(|dword| nic.peek(regs::MTA + dword * 4)).collect(); + + fill(&nic, &mut driver, 1); + nic.set_link(false); + if !seen_down { + // Down and up again between two passes: one `LSC`, and a link that + // reads up on both sides of it. + nic.set_link(true); + } + let pass = one_pass(&mut driver); + assert!(pass.rearmed, "{}", why("the ring was left with the link's unsent frames")); + assert_eq!(driver.counters().unsent, (TX_RING - 1) as u32, "{}", why("the frames given back were not counted")); + if seen_down { + assert_eq!(driver.tx_room(), 0, "{}", why("a link that is down had room")); + } + if driver.link().is_up() { + // The reset that left the PHY alone has its link back by the + // bring-up's own `SLU`, which is a link change of the driver's + // making: a frame in flight at the next pass is not given back + // for it. + let early = frame(0x70, 64); + let slot = driver.tx_reserve(early.len()).expect("a ring with room"); + nic.put_bytes(slot.at, &early); + driver.tx_commit(slot); + let next = one_pass(&mut driver); + assert!(!next.rearmed, "{}", why("the bring-up's own link change took the ring back again")); + nic.run(); + assert_eq!(nic.sent(), vec![early], "{}", why("the frame published after the reset did not leave")); + reached_at_once += 1; + } + + link_up(&nic, &mut driver); + assert_eq!(driver.tx_room(), TX_RING - 1, "{}", why("the ring did not come back whole")); + let after: Vec = (0..regs::MTA_DWORDS).map(|dword| nic.peek(regs::MTA + dword * 4)).collect(); + assert_eq!(after, table, "{}", why("the multicast table did not survive")); + + // A frame published now leaves, and it is the only one that does: the + // pass that follows a frame in flight resets nothing. + let payload = frame(0x77, 300); + let slot = driver.tx_reserve(payload.len()).expect("a ring with room"); + nic.put_bytes(slot.at, &payload); + driver.tx_commit(slot); + let next = one_pass(&mut driver); + assert!(!next.rearmed, "{}", why("the bring-up's own link change took the ring back again")); + let inbound = frame(0x78, 200); + nic.deliver(&inbound); + nic.run(); + assert_eq!(nic.sent(), vec![payload], "{}", why("what left is not the one frame published since")); + one_pass(&mut driver); + assert_eq!(drain(&nic, &mut driver), vec![inbound], "{}", why("the receive ring did not come back")); + assert_eq!(driver.counters().unsent, (TX_RING - 1) as u32); + } + assert!(reached_at_once > 0, "no part had its link back at the end of the reset"); +} + +/// A function that does not come back from that reset is handed up by name, +/// and not driven on. +#[test] +fn a_ring_that_cannot_be_taken_back_is_refused() { + let (nic, mut driver) = held_with_a_link(27, Part::E82574, Permits::default()); + fill(&nic, &mut driver, 1); + nic.set_link(false); + nic.reset_never_clears(); + let Err(PassRefused::Rearm(Refusal::ResetUnfinished { .. })) = driver.begin_pass() else { + panic!("{}", nic.because("a reset that never cleared began a pass")); + }; +} + /// §7.2.4.2 lets the device hold transmit write-backs in a batch, so a later /// descriptor's `DD` says nothing about an earlier one's: the ring is /// reclaimed from the oldest, one at a time, stopping at the first that is @@ -785,6 +911,7 @@ fn transmit_descriptors_are_reclaimed_under_batched_write_back() { let nic = Nic::new(seed); let mut driver = open(&nic); nic.set_link(true); + one_pass(&mut driver); let mut out: Vec> = Vec::new(); let mut on_the_wire: Vec> = Vec::new(); for i in 0..(TX_RING as u8 * 4) { @@ -811,6 +938,7 @@ fn transmit_descriptors_are_reclaimed_under_batched_write_back() { "{}", nic.because("what reached the wire is not what was published") ); + assert_eq!(out.len(), TX_RING * 4, "{}", nic.because("a frame found no descriptor in eight runs")); } } @@ -959,7 +1087,7 @@ fn a_claim_that_stops_answering_is_handed_up_and_not_read_as_quiet() { nic.set_link(true); one_pass(&mut driver); nic.claim_taken_away(); - assert_eq!(driver.begin_pass(), Err(Unanswered::Gone)); + assert_eq!(driver.begin_pass(), Err(PassRefused::Claim(Unanswered::Gone))); } /// §10.2.4.1's `RXO` and `RXDMT0`: the two the part reports about its own @@ -984,8 +1112,8 @@ fn a_frame_longer_than_a_transmit_buffer_is_refused_and_not_truncated() { let nic = Nic::new(22); let mut driver = open(&nic); nic.set_link(true); + one_pass(&mut driver); assert!(driver.tx_reserve(TX_BUF_BYTES + 1).is_none()); - assert_eq!(driver.counters().too_long, 1); // And the ring is untouched: the next frame that does fit still goes out. let payload = frame(0x99, TX_BUF_BYTES); let slot = driver.tx_reserve(payload.len()).expect("a free transmit descriptor"); @@ -1043,6 +1171,7 @@ fn the_link_going_away_and_coming_back_is_seen() { let down = one_pass(&mut driver); assert!(down.causes & cause::LSC != 0, "{}", nic.because("no LSC for the link going away")); assert!(down.link_changed); + assert!(!down.rearmed, "{}", nic.because("a link change over an empty ring reset the function")); assert_eq!(driver.link(), Link::Down); nic.set_link(true); @@ -1099,11 +1228,20 @@ fn a_seeded_workload_loses_nothing_and_invents_nothing() { let mut got_out: Vec> = Vec::new(); for round in 0..40u8 { - let inbound = frame(round + 1, 64 + round as usize * 13); - nic.deliver(&inbound); - expect_in.push(inbound); if round % 5 == 3 { + // A frame the part still holds when its link changes goes with + // the ring the driver takes back, so the wire is quiet first. + for _ in 0..8 { + nic.run(); + one_pass(&mut driver); + got_in.extend(drain(&nic, &mut driver)); + got_out.extend(nic.sent()); + } nic.set_link(round % 10 == 3); + } else { + let inbound = frame(round + 1, 64 + round as usize * 13); + nic.deliver(&inbound); + expect_in.push(inbound); } let outbound = frame(0x80 | round, 90 + round as usize * 11); if let Some(slot) = driver.tx_reserve(outbound.len()) { @@ -1112,7 +1250,12 @@ fn a_seeded_workload_loses_nothing_and_invents_nothing() { expect_out.push(outbound); } nic.run(); + let given_back = driver.counters().unsent; one_pass(&mut driver); + // Published into a link that had gone and the driver had not yet + // seen go: the newest frames, counted and never sent. + let given_back = (driver.counters().unsent - given_back) as usize; + expect_out.truncate(expect_out.len() - given_back); got_in.extend(drain(&nic, &mut driver)); got_out.extend(nic.sent()); } @@ -2687,6 +2830,7 @@ fn the_driver_and_the_macs_statistics_count_the_same_frames() { let nic = Nic::new(134); let mut driver = open(&nic); nic.set_link(true); + one_pass(&mut driver); for tag in 1..=3u8 { let payload = frame(tag, 100); let slot = driver.tx_reserve(payload.len()).expect("a free transmit descriptor"); diff --git a/userland/netstack/src/card.rs b/userland/netstack/src/card.rs index 25f2c8d68f9..88e5d1e5307 100644 --- a/userland/netstack/src/card.rs +++ b/userland/netstack/src/card.rs @@ -63,14 +63,16 @@ impl Card { /// silently never arrives, so this dies where it can be read — once, for /// whichever driver is running. /// + /// So does an Intel part that did not come back from the reset a link + /// change over unsent frames costs it: nothing drives it from there. + /// /// Answers the link where the pass found it changed; virtio reports none. pub fn begin_pass(&self) -> Option { let answered = match self { - Self::Virtio(nic) => nic.take_interrupt().map(|_| None), + Self::Virtio(nic) => nic.take_interrupt().map(|_| None).map_err(toyos_i219::PassRefused::Claim), Self::Intel(nic) => nic.begin_pass(), }; - answered - .unwrap_or_else(|why| panic!("netstack: this NIC's claim refused an interrupt read: {why:?}")) + answered.unwrap_or_else(|why| panic!("netstack: this NIC cannot be driven on — {why}")) } /// What `inspect` reads about the card: which driver, its address, its @@ -118,6 +120,10 @@ impl Card { let (counters, wire) = nic.counts(); snap.put("descriptors.sent", counters.sent); snap.put("descriptors.received", counters.received); + snap.put("descriptors.unsent", counters.unsent); + snap.put("transmit.full", counters.tx_full); + snap.put("transmit.wake_armed", counters.tx_wake_armed); + snap.put("transmit.wake_taken", counters.tx_wake_taken); snap.put("wire.sent", wire.sent); snap.put("wire.received", wire.received); snap.put("wire.seen", wire.seen); diff --git a/userland/netstack/src/i219.rs b/userland/netstack/src/i219.rs index 48914e85e6a..e051e95cf23 100644 --- a/userland/netstack/src/i219.rs +++ b/userland/netstack/src/i219.rs @@ -292,8 +292,18 @@ impl Nic { /// Take the interrupt, acknowledge its causes and refresh the link — and /// answer the link where this pass found it changed. - pub fn begin_pass(&self) -> Result, SyscallError> { + /// + /// A pass that reset the function to take its transmit ring back says so, + /// with what the bring-up found the second time. + pub fn begin_pass(&self) -> Result, toyos_i219::PassRefused> { let pass = self.driver.borrow_mut().begin_pass()?; + if pass.rearmed { + crate::say!( + "netstack: I219: the link changed over unsent frames, and the function was reset \ + to take its transmit ring back" + ); + say_brought_up(self.driver.borrow().brought_up()); + } Ok(pass.link_changed.then(|| self.link())) } @@ -330,9 +340,9 @@ impl Nic { /// Fill a transmit buffer with a `len`-byte frame and hand it to the /// device. /// - /// Non-blocking, and for a caller [`Self::tx_room`] answered: a frame the - /// driver has no slot for is a caller that did not ask, or one that offered - /// more than a buffer holds. + /// Non-blocking, and for a caller [`Self::tx_room`] answered in this + /// pass: a frame the driver has no slot for is a caller that did not ask, + /// or one that offered more than a buffer holds. pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { let slot = self.driver.borrow_mut().tx_reserve(len).unwrap_or_else(|| { panic!("netstack: a {len}-byte frame was offered to a transmit ring that refuses it") @@ -380,13 +390,14 @@ impl Nic { crate::say!( "netstack: I219: refused {} over-length, {} errored, {} split and {} empty \ descriptor(s); {} overrun(s), {} descriptor-starvation report(s), \ - {} spurious interrupt(s)", + {} frame(s) given back unsent at a link change, {} spurious interrupt(s)", counters.over_length, counters.errored, counters.split, counters.empty, counters.overruns, counters.starved, + counters.unsent, counters.spurious, ); } diff --git a/userland/netstack/src/virtio_net.rs b/userland/netstack/src/virtio_net.rs index 5f31eaff840..759dc15266f 100644 --- a/userland/netstack/src/virtio_net.rs +++ b/userland/netstack/src/virtio_net.rs @@ -339,7 +339,6 @@ fn config_space_is_bounded(dev: &PciDev) -> Result<(), Refusal> { pub struct VirtioNet { dev: PciDev, rx_doorbell: Doorbell, - tx_doorbell: Doorbell, /// Held for their mappings' lives: every window above and below points into /// one of these two. _bar: SharedMemory, @@ -349,9 +348,7 @@ pub struct VirtioNet { /// what the unit translates for this function and for nothing else. dma_device_addr: u64, rx: RefCell, - tx: RefCell, - /// Transmit heads nothing is in flight on. - tx_free: RefCell>, + tx: RefCell, reported: Latch, mac: [u8; 6], } @@ -454,15 +451,7 @@ impl VirtioNet { dma.sub(OFF_RX_USED, USED_RING_OFF + RX_QUEUE_SIZE as usize * USED_ELEM_BYTES), RX_QUEUE_SIZE, ); - let mut tx = Rings::new( - dma.sub(OFF_TX_RINGS, TX_QUEUE_SIZE as usize * DESC_BYTES), - dma.sub(OFF_TX_RINGS + tx_avail_off(), AVAIL_RING_OFF + TX_QUEUE_SIZE as usize * 2), - dma.sub( - OFF_TX_RINGS + tx_used_off(), - USED_RING_OFF + TX_QUEUE_SIZE as usize * USED_ELEM_BYTES, - ), - TX_QUEUE_SIZE, - ); + let mut tx = tx_rings(dma); setup_queue(common, RX_QUEUE, &mut rx, dma_device_addr, OFF_RX_DESC, OFF_RX_AVAIL, OFF_RX_USED)?; setup_queue( @@ -510,14 +499,12 @@ impl VirtioNet { let nic = Self { dev, rx_doorbell, - tx_doorbell, _bar: mapped, _grant: grant, dma, dma_device_addr, rx: RefCell::new(rx), - tx: RefCell::new(tx), - tx_free: RefCell::new((0..TX_QUEUE_SIZE).rev().collect()), + tx: RefCell::new(TxQueue::new(tx, dma, dma_device_addr, tx_doorbell)), reported: Latch::default(), mac, }; @@ -536,7 +523,7 @@ impl VirtioNet { /// Say what this driver has refused, when the count has moved. Once a /// pass, never per element: a burst of refusals is one line. pub fn report(&self) { - let refused = self.rx.borrow().refused + self.tx.borrow().refused; + let refused = self.rx.borrow().refused + self.tx.borrow().rings.refused; if self.reported.moved(refused).is_none() { return; } @@ -616,68 +603,102 @@ impl VirtioNet { unsafe { std::slice::from_raw_parts(window.as_ptr() as *const u8, len) } } - /// How many frames the transmit queue takes now, every head the device - /// has finished with taken back first. + /// How many frames the transmit queue takes now ([`TxQueue::room`]). A + /// caller answered 0 sleeps on the claim. + pub fn tx_room(&self) -> usize { + self.tx.borrow_mut().room() + } + + /// Fill a transmit buffer with a `len`-byte frame and hand it to the + /// device ([`TxQueue::send`]). + pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { + self.tx.borrow_mut().send(len, fill) + } + + /// The claim, for the poller: readable means an interrupt has landed. + pub fn claim(&self) -> &PciDev { + &self.dev + } +} + +/// The transmit queue's three rings, where the grant's layout puts them. +fn tx_rings(dma: Window) -> Rings { + Rings::new( + dma.sub(OFF_TX_RINGS, TX_QUEUE_SIZE as usize * DESC_BYTES), + dma.sub(OFF_TX_RINGS + tx_avail_off(), AVAIL_RING_OFF + TX_QUEUE_SIZE as usize * 2), + dma.sub(OFF_TX_RINGS + tx_used_off(), USED_RING_OFF + TX_QUEUE_SIZE as usize * USED_ELEM_BYTES), + TX_QUEUE_SIZE, + ) +} + +/// The transmit queue: its rings, the heads nothing is in flight on, and the +/// buffer each head owns. Everything in it is memory, so the host drives it +/// with a plain allocation for the grant. +struct TxQueue { + rings: Rings, + /// Transmit heads nothing is in flight on. + free: Vec, + /// The grant, and where the device reaches its first byte. + dma: Window, + dma_device_addr: u64, + doorbell: Doorbell, +} + +impl TxQueue { + fn new(rings: Rings, dma: Window, dma_device_addr: u64, doorbell: Doorbell) -> Self { + Self { rings, free: (0..TX_QUEUE_SIZE).rev().collect(), dma, dma_device_addr, doorbell } + } + + /// How many frames the queue takes now, every head the device has + /// finished with taken back first. /// /// **Room returning is an interrupt already**: this driver negotiates no /// `VIRTIO_F_EVENT_IDX` and leaves the transmit queue's `avail.flags` 0, /// and §2.7.7 has the device notify for every buffer it uses on such a - /// queue. A caller answered 0 sleeps on the claim. - pub fn tx_room(&self) -> usize { - self.reclaim_tx(); - self.tx_free.borrow().len() + /// queue. + fn room(&mut self) -> usize { + while let Some((head, _)) = self.rings.poll_used() { + self.free.push(head); + } + self.free.len() } /// Fill a transmit buffer with a `len`-byte frame and hand it to the device. /// /// **The buffer is the head's own and the two are taken together**, so /// nothing is written into a buffer the device is reading: a head leaves - /// `tx_free` only here and comes back only in [`Self::reclaim_tx`], which - /// the device's used ring is what drives. + /// `free` only here and comes back only in [`Self::room`], which the + /// device's used ring is what drives. /// - /// Non-blocking, and for a caller [`Self::tx_room`] answered: a frame + /// Non-blocking, and for a caller [`Self::room`] answered: a frame /// offered with no head free is a caller that did not ask. - pub fn tx(&self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { + fn send(&mut self, len: usize, fill: impl FnOnce(&mut [u8]) -> R) -> R { assert!( NET_HDR_SIZE + len <= TX_BUF_SIZE, "netstack: a {len}-byte frame does not fit this NIC's transmit buffer" ); let head = self - .tx_free - .borrow_mut() + .free .pop() .expect("netstack: a frame was offered to a transmit queue that had said it has no room"); let at = OFF_TX_BUFS + head as usize * TX_BUF_SIZE; // The header is this driver's and zeroed before the frame goes in. self.dma.sub(at, NET_HDR_SIZE).zero(); let window = self.dma.sub(at + NET_HDR_SIZE, len); - // SAFETY: the window is inside the grant, which lives as long as - // `self`; the device is not reading it, because this head is out of - // `tx_free` and its descriptor is published only after `fill` returns. + // SAFETY: the window is inside the grant, which lives as long as the + // driver that holds this queue; the device is not reading it, because + // this head is out of `free` and its descriptor is published only + // after `fill` returns. let result = fill(unsafe { std::slice::from_raw_parts_mut(window.as_ptr(), len) }); - self.tx.borrow_mut().submit( + self.rings.submit( head, self.dma_device_addr + at as u64, (NET_HDR_SIZE + len) as u32, false, - self.tx_doorbell, + self.doorbell, ); result } - - /// Take back every transmit head the device has finished with. - fn reclaim_tx(&self) { - loop { - let done = self.tx.borrow_mut().poll_used(); - let Some((head, _)) = done else { return }; - self.tx_free.borrow_mut().push(head); - } - } - - /// The claim, for the poller: readable means an interrupt has landed. - pub fn claim(&self) -> &PciDev { - &self.dev - } } /// Where a queue's doorbell is, from `queue_notify_off` and the multiplier the @@ -800,6 +821,85 @@ mod tests { Rings::new(window.sub(0, 0x400), window.sub(0x400, 0x400), window.sub(0x800, 0x400), size) } + /// A transmit queue over a plain allocation the size of the grant, and + /// the doorbell a write to which nothing hears. + fn queue() -> TxQueue { + let grant = vec![0u8; GRANT_BYTES as usize].leak(); + let bell = vec![0u8; 2].leak(); + // SAFETY: `leak` gives both allocations the `'static` lifetime the + // windows need, and nothing but this queue and the test reaches them. + let (dma, bell) = unsafe { + (Window::new(grant.as_mut_ptr(), grant.len()), Window::new(bell.as_mut_ptr(), bell.len())) + }; + TxQueue::new(tx_rings(dma), dma, 0x4000_0000, Doorbell { window: bell, queue: TX_QUEUE }) + } + + /// The device, finishing with the `count` oldest chains it was given and + /// answering their heads (§2.7.8). + fn device_uses(queue: &TxQueue, count: u16) -> Vec { + let rings = &queue.rings; + let used_idx: u16 = rings.used.read(USED_IDX_OFF); + (0..count) + .map(|nth| { + let at = used_idx.wrapping_add(nth); + let head: u16 = rings.avail.read(AVAIL_RING_OFF + (at % rings.size) as usize * 2); + let element = USED_RING_OFF + (at % rings.size) as usize * USED_ELEM_BYTES; + rings.used.write(element, head as u32); + rings.used.write(element + 4, 0u32); + rings.used.write(USED_IDX_OFF, at.wrapping_add(1)); + head + }) + .collect() + } + + /// The queue says how many frames it takes before one is offered: every + /// head in flight is no room, and room returns by exactly the heads the + /// device has finished with. Nothing is dropped on the way: each frame + /// published is in its own buffer behind a zeroed header. + #[test] + fn a_full_transmit_queue_has_no_room_until_the_device_gives_heads_back() { + let mut queue = queue(); + let mut published = 0u8; + while queue.room() > 0 { + assert!(published < TX_QUEUE_SIZE as u8 * 2, "the transmit queue never filled"); + queue.send(60, |frame| frame.fill(published + 1)); + published += 1; + } + assert_eq!(published as u16, TX_QUEUE_SIZE); + let avail_idx: u16 = queue.rings.avail.read(AVAIL_IDX_OFF); + assert_eq!(avail_idx, TX_QUEUE_SIZE); + for nth in 0..TX_QUEUE_SIZE { + let head: u16 = queue.rings.avail.read(AVAIL_RING_OFF + nth as usize * 2); + let chain: Desc = queue.rings.desc.read(head as usize * DESC_BYTES); + assert_eq!(chain.len as usize, NET_HDR_SIZE + 60); + let buffer = queue.dma.sub((chain.addr - queue.dma_device_addr) as usize, chain.len as usize); + let bytes: Vec = (0..chain.len as usize).map(|at| buffer.read::(at)).collect(); + assert_eq!(bytes[..NET_HDR_SIZE], [0; NET_HDR_SIZE]); + assert_eq!(bytes[NET_HDR_SIZE..], [nth as u8 + 1; 60]); + } + + let back = device_uses(&queue, 3); + assert_eq!(queue.room(), 3); + // The next frame goes out on a head the device gave back, and on no + // head still in flight. + queue.send(60, |frame| frame.fill(0xEE)); + let reused: u16 = queue.rings.avail.read(AVAIL_RING_OFF + (TX_QUEUE_SIZE % queue.rings.size) as usize * 2); + assert!(back.contains(&reused)); + assert_eq!(queue.room(), 2); + } + + /// A frame offered to a queue that said it had no room is netstack's own + /// bug, and dies by name instead of being written somewhere and dropped. + #[test] + #[should_panic(expected = "had said it has no room")] + fn a_frame_offered_to_a_full_transmit_queue_is_not_taken() { + let mut queue = queue(); + while queue.room() > 0 { + queue.send(60, |frame| frame.fill(1)); + } + queue.send(60, |frame| frame.fill(2)); + } + #[test] fn a_head_past_the_table_is_refused() { let queue = rings(16); From 61b00f16cde4cd3c19bbcb211c89e0ec45aff2b2 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 00:22:59 +0200 Subject: [PATCH 4/7] Review round 2 of #782: a wake is a message for the transmit cause alone, and the I219 is never reset over a ring it holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `transmit.wake_taken` counted any pass that read the transmit cause while it was unmasked, and §7.4.3 keeps a cause in `ICR` whether or not a message was raised for it: a part that raises nothing for an unmasked `TXDW` still met the hardware issue's exit. A wake is now a pass a message began whose only unmasked cause is the transmit one. The test was red at 12e4fb269 with `(1, 1, 1)` where it wants `(1, 1, 0)`. The re-arm reset the function exactly when its transmit ring was not empty, on the 82574's two sentences. Nothing Intel publishes that could be found says that is safe on the PCH's MAC, and nothing says what the sequence would be: the I219 and PCH datasheets, five PCH specification updates and the I218's are silent on a reset over published descriptors. So the I219 is no longer reset there. The descriptors a link change finds unsent stay the part's, counted `descriptors.stranded`; it is owed their write-back within the slowest time §10.2.6.1 lets a full ring leave, from the pass that read the link up; the ring goes on when they come, and a part that has not written them back is refused by name. netstack takes that deadline into its wait, since a part that never writes back sends nothing to say so. The 82574 keeps its reset, and its document turned out to say what it does otherwise: the Defer Count counts a transmit deferred because "The link is not up". The reset takes the MAC's statistics, so the re-arm reads them first. The link is refreshed on `LSC` alone: the cause is recorded masked or not, so the arm kept for a link change that raises none had no case. `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md` records what is unknown of the I219 and that `open`'s reset has the same exposure. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...-read-an-intel-nic-across-a-link-change.md | 69 ++++++++ ...nst-the-82574-datasheets-newer-revision.md | 4 +- ...cs-room-and-wake-are-unread-on-hardware.md | 27 ++- toyos-i219/src/lib.rs | 160 +++++++++++++---- toyos-i219/src/stub.rs | 20 ++- toyos-i219/src/tests.rs | 161 ++++++++++++++++-- userland/netstack/src/card.rs | 17 +- userland/netstack/src/i219.rs | 12 +- userland/netstack/src/main.rs | 2 + 9 files changed, 409 insertions(+), 63 deletions(-) create mode 100644 issues/no-machine-has-read-an-intel-nic-across-a-link-change.md diff --git a/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md new file mode 100644 index 00000000000..d8297eb1eaf --- /dev/null +++ b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md @@ -0,0 +1,69 @@ +--- +status: open +kind: tooling +opened: 2026-10-09 +--- + +# No machine has read an Intel NIC across a link change, and two things about the I219 there are unknown + +A link that changes while the transmit ring holds unsent descriptors is +answered per part (`toyos-i219/src/lib.rs`, "A frame is offered only to a ring +with room for it"), and every reading of either answer is the driver's own +model. + +**The 82574** is reset and brought up again, by its own document: §10.2.6.1 +("Software should combine this with a reset if the packets in the FIFO need to +be flushed") and §10.2.6.7. The same document's Defer Count (§10.2.7) says the +part would otherwise have kept those frames for the link that returns. No +82574 is in reach, and QEMU's `e1000e` is not in the harness. + +**The I219 is not reset over a ring it holds descriptors in**, because two +things are unknown of it: + +- **Whether it writes those descriptors back when the link returns.** The + 82574's Defer Count is the only sentence on the subject in any document + read, and it is the other part's. The driver gives the I219 the slowest + time that document lets a full ring leave (`STRANDED_DEADLINE_NANOS`, + 12.98 s) from the link being read up, and then refuses it by name: netstack + ends with "cannot be driven on". +- **Whether a reset over a live ring hangs it.** Nothing Intel publishes that + was found says it is safe or that it is not. Read, and silent on it: the + I219 datasheet (612523 rev 2.02), the 500 Series on-package PCH datasheet + volumes 1 and 2 (631119-007 §18, 631120-002 §8), the 100 Series PCH + datasheet volume 2 (332691) and specification update (332692-015), the 400 + Series PCH datasheet volume 2 (620855-002) and specification update + (620856-009), the 300 Series PCH specification update (337349-012), the 7th + generation U/Y platform I/O specification update (334660-009), the I218 + specification update (rev 1.0), and the 82574 datasheet (317694 rev 3.4) and + specification update. intel.com's search finds no I219 specification + update, sighting or application note, and no specification update for the + 500 Series on-package PCH. The one piece of evidence the tree has is + `issues/the-t14-hung-after-rebooting-with-its-i219-faulted.md`: a T14 that + froze at reboot with that function left holding live rings, cause not + established. + +**`open`'s own reset has the same exposure.** Every start of netstack resets +the function before it programs it. On a netstack started again over a +function that still holds published descriptors, `toyos_i219::quiesce` stops +the receiver and the transmitter and the reset then goes out over that ring. +Nothing guards it, because nothing found says what the guard is. + +## Owner + +Stage 2 of `issues/the-lan-is-not-yet-production-grade.md`. + +## Exit + +None can be read yet without a hand on the cable: a link change under +traffic on the T14 is its cable pulled or its switch port cycled, and no row +can do either. It becomes a row's when the T14's link partner is something a +row commands. That row fills the ring, drops the link and raises it, and +reads `descriptors.stranded` above 0 with netstack still serving (the part +writes them back) or netstack's "cannot be driven on" line naming the ring +(it does not, and the driver's answer is then a document or an owner's ruling +away). The reset over a live ring is read by a row that starts netstack again +under traffic and reboots: the machine comes back, or it does not. + +An Intel document that states the I219's sequence for a reset over published +descriptors closes the second unknown without a machine: the driver then +does exactly that, in `open` as well. diff --git a/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md b/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md index abe7976efe0..ed131f2c240 100644 --- a/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md +++ b/issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md @@ -24,7 +24,9 @@ every other section the tree cites that was compared sits at the same number. Neither has been seen to matter: the T14 has sent frames with both. Whether either register means the same on the PCH's MAC no document says (631120 §8.2 -publishes neither). +publishes neither). The transmit wake rests on a write-back per descriptor +with bit 22 clear, and on the I219 the reading of that is the T14 boot +`issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md` exits on. ## Owner diff --git a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md index 020c23d87c8..2f2847a279f 100644 --- a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md +++ b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md @@ -22,12 +22,18 @@ interrupt register of that MAC at all. A wake that never comes is no wedge and no lost frame: room is counted from the descriptors in memory, so the next pass any other event begins finds it. It is a stall until that pass, and it reads as `transmit.wake_armed` ahead of -`transmit.wake_taken`. +`transmit.wake_taken`: the driver counts a wake only on a pass a message +began whose one unmasked cause was the transmit one, so a cause the part +recorded and raised nothing for is never counted. -Not in the exit: the reset that takes the transmit ring back when the link -changes over unsent frames. It needs a link to drop under traffic, which no -row can stage. A boot where it happens says so by itself: netstack's line -"the link changed over unsent frames", and `descriptors.unsent` in `inspect`. +The same boot is the reading of `TXDCTL` as the bring-up writes it, with bit 22 +clear and a write-back per descriptor +(`issues/the-intel-driver-writes-txdctl-and-tidv-against-the-82574-datasheets-newer-revision.md`): +`descriptors.sent` short of `wire.sent`, or armed ahead of taken, makes that +bit this path's. + +What either part does with its ring across a link change is not in this +exit: `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md`. ## Owner @@ -36,8 +42,11 @@ first boot that gives netstack the wired card. ## Exit -A T14 boot whose netstack drives `8086:15fc` sends more than -`toyos_i219::TX_RING - 1` frames in one pass, and `inspect` then answers +A T14 boot whose netstack drives `8086:15fc` queues more than +`toyos_i219::TX_RING - 1` frames in one pass, from more sockets than one and +to a destination that answers none of them, and `inspect` then answers `transmit.full`, `transmit.wake_armed` and `transmit.wake_taken` each at least -1, `descriptors.unsent` 0, and `descriptors.sent` equal to `wire.sent`; the -boot's log carries no "cannot be driven on" line from netstack. +1, `descriptors.unsent` and `descriptors.stranded` 0, and `descriptors.sent` +equal to `wire.sent`; the boot's log carries no "cannot be driven on" line +from netstack. A boot whose `transmit.full` reads 0 never filled the ring and +has read nothing. diff --git a/toyos-i219/src/lib.rs b/toyos-i219/src/lib.rs index e68e25c5f8b..525d218ca37 100644 --- a/toyos-i219/src/lib.rs +++ b/toyos-i219/src/lib.rs @@ -38,14 +38,25 @@ //! cause that says a descriptor came back. Nothing here drops a frame for want //! of a descriptor, and nothing waits on the part. //! -//! **A link that is down has no room, and a link that changed gives the ring -//! back.** No Intel document says what either part does with a descriptor it -//! holds when its link goes away, so nothing here depends on it: no frame is -//! published while the link is down, and a pass that reads `LSC` with a -//! descriptor still unsent resets the function and brings it up again -//! ([`I219::begin_pass`]) — §10.2.6.7 lets software write the ring's head -//! only "after a reset (hardware reset or CTRL.RST) and before enabling the -//! transmit function", so that reset is the one way a ring is taken back. +//! **A link that is down has no room**, and no frame is published while it +//! is. What the 82574 does with one it already holds is in §10.2.7's Defer +//! Count: "A defer event occurs when the transmitter cannot immediately send a +//! packet due to the medium being busy either because: [...] The link is not +//! up" — it keeps the frame for the link that comes. A pass that reads `LSC` +//! with a descriptor still unsent ([`I219::begin_pass`]) answers per part: +//! +//! - **The 82574 gives the ring back**, because a frame queued for a link that +//! has gone is not one for the link that came after it: the function is +//! reset and brought up again. §10.2.6.1 on `TCTL.EN`: "Software should +//! combine this with a reset if the packets in the FIFO need to be flushed", +//! and §10.2.6.7 lets software write the ring's head only "after a reset +//! (hardware reset or CTRL.RST) and before enabling the transmit function". +//! - **The PCH's MAC is never reset over a ring it holds descriptors in.** No +//! Intel document says that is safe on that part and none says what it does +//! with those descriptors, so they stay its own: it is owed their write-back +//! within `STRANDED_DEADLINE_NANOS` of the link being read up over them, the +//! ring goes on when they come, and a part that has not written them back by +//! then is refused by name ([`PassRefused::Stranded`]) and driven no further. //! //! # The device is not trusted //! @@ -411,6 +422,9 @@ pub enum PassRefused { /// The function did not come back up from the reset that takes its /// transmit ring back. Rearm(Refusal), + /// The PCH's MAC still holds `left` of the descriptors a link change left + /// in its transmit ring, this long after the link was read up over them. + Stranded { left: usize, after_nanos: u64 }, } impl core::fmt::Display for PassRefused { @@ -421,6 +435,12 @@ impl core::fmt::Display for PassRefused { f, "it did not come back from the reset that takes its transmit ring back: {why}" ), + Self::Stranded { left, after_nanos } => write!( + f, + "a link change left its transmit ring holding frames, and {} ms after the link \ + returned it has not written {left} of their descriptor(s) back", + after_nanos / 1_000_000 + ), } } } @@ -447,12 +467,16 @@ pub struct Counters { pub tx_full: u32, /// Times the transmit cause was unmasked for such a caller. pub tx_wake_armed: u32, - /// Passes that read the transmit cause while it was unmasked: the wake a - /// full ring was waiting on, taken. + /// Passes a message began whose only unmasked cause was the transmit one, + /// while it was unmasked: the wake a full ring was waiting on, taken, and + /// nothing a pass begun by anything else reads. pub tx_wake_taken: u32, - /// Frames published and never sent: the descriptors a link change gave - /// back ([`I219::begin_pass`]). + /// Frames published and never sent: the descriptors the 82574's reset + /// gave back at a link change ([`I219::begin_pass`]). pub unsent: u32, + /// Descriptors a link change found unsent in the PCH MAC's ring, and left + /// there for it to write back. + pub stranded: u32, /// Messages that arrived with no cause set in `ICR` — §7.4.5's spurious /// interrupt. It costs a pass and nothing else. pub spurious: u32, @@ -523,6 +547,18 @@ const RESET_SETTLE_NANOS: u64 = 1_000; /// the PCIe Master Enable Status bit is not cleared within a given time". const MASTER_QUIESCE_DEADLINE_NANOS: u64 = 10_000_000; +/// How long the PCH's MAC has to write back the descriptors a link change left +/// in its transmit ring, from the pass that read the link up over them. +/// +/// **A ceiling on an event, and the slowest §10.2.6.1 lets a full ring leave**: +/// [`TX_RING`]` - 1` frames of a whole buffer each, on a half-duplex link at +/// 10 Mb/s, where a byte is 800 ns; each sent the "total of 16 attempts" a +/// `TCTL.CT` of 15 allows, and each attempt behind a back-off that "clamps to +/// the maximum number of slot times after 10 retries" — ten doublings of a +/// slot, which is the collision window of "64 bytes for 10/100 Mb/s". +const STRANDED_DEADLINE_NANOS: u64 = + (TX_RING as u64 - 1) * 16 * (TX_BUF_BYTES as u64 + (1 << 10) * 64) * 800; + /// The transmit control [`I219::open`] writes: §4.6.6's suggested values with /// the transmitter enabled. const TX_CONTROL: u32 = tctl::EN | tctl::PSP | tctl::CT | tctl::COLD_FULL_DUPLEX; @@ -787,6 +823,9 @@ pub struct I219 { /// Whether [`Part::tx_done`] is unmasked: a caller found the ring full /// and no pass has begun since. tx_wake: bool, + /// The descriptors a link change left in the PCH MAC's transmit ring that + /// it has not yet written back. + stranded: Option, counters: Counters, /// What [`Self::wire`] has read out of the statistics registers so far. wire: Wire, @@ -800,6 +839,16 @@ impl Drop for I219 { } } +/// The oldest descriptors of the transmit ring, published before a link change +/// and not written back since. +#[derive(Clone, Copy)] +struct Stranded { + left: usize, + /// When the link change was read, which the deadline runs from while the + /// link is up. + since: u64, +} + /// What [`program`] leaves beside the registers it wrote. struct Programmed { mac: [u8; 6], @@ -1046,6 +1095,7 @@ impl I219 { tx_next: 0, tx_clean: 0, tx_wake: false, + stranded: None, counters: Counters::default(), wire: Wire::default(), }; @@ -1125,11 +1175,18 @@ impl I219 { /// function is no longer this driver's, and the refusal is handed up rather /// than counted as quiet. /// - /// **A pass that reads `LSC` with a transmit descriptor still unsent takes - /// the ring back**, by the reset and the bring-up [`Self::open`] runs: - /// the frames in it are counted [`Counters::unsent`], a frame received and - /// not yet handed up goes with them, and the pass lasts as long as a - /// bring-up does. Every [`Frame`] is given back before a pass begins. + /// **On the 82574 a pass that reads `LSC` with a transmit descriptor still + /// unsent takes the ring back**, by the reset and the bring-up + /// [`Self::open`] runs: the frames in it are counted [`Counters::unsent`], + /// a frame received and not yet handed up goes with them, and the pass + /// lasts as long as a bring-up does. Every [`Frame`] is given back before + /// a pass begins. `LSC` says the link changed and not that it is down, so + /// a link that reads up on both sides of it costs the same. + /// + /// **On the PCH's MAC those descriptors stay the part's**, counted + /// [`Counters::stranded`], and a pass that finds one still not written + /// back `STRANDED_DEADLINE_NANOS` after the link was read up over them + /// is refused. [`Self::pass_due_in`] is when that pass has to begin. pub fn begin_pass(&mut self) -> Result> { self.rx_budget = RX_BUDGET; let messages = match self.irq.taken() { @@ -1168,24 +1225,62 @@ impl I219 { if messages > 0 && acknowledged == 0 { self.counters.spurious = self.counters.spurious.saturating_add(messages); } - if waited && causes & self.part.tx_done() != 0 { + // §7.4.3: "The cause bit stores the interrupt event regardless of the + // state of the mask bit", so the transmit cause is in `ICR` on a pass + // anything began. It is the wake only where a message came and no + // other unmasked cause is there to have raised it. + if waited + && messages > 0 + && causes & self.part.tx_done() != 0 + && causes & (cause::ENABLED | cause::ENABLED_MSIX) == 0 + { self.counters.tx_wake_taken = self.counters.tx_wake_taken.saturating_add(1); } - // On `LSC` and on every pass that found no cause at all: the link can - // also come up before the mask was written, and then no `LSC` is ever - // delivered for it. + // On `LSC` alone: §7.4.3 records it masked or not, so a link that came + // up before the mask was written is in this read all the same. let before = self.link; - if causes & cause::LSC != 0 || !self.link.is_up() { + let mut rearmed = false; + if causes & cause::LSC != 0 { self.refresh_link(); + match (self.unsent(), self.part) { + (0, _) => {} + (_, Part::E82574) => { + self.rearm().map_err(PassRefused::Rearm)?; + rearmed = true; + } + (unsent, Part::I219) => { + // A second change over the same descriptors counts none of + // them twice, and the deadline runs from the newest. + let counted = self.stranded.map_or(0, |stranded| stranded.left); + self.counters.stranded = + self.counters.stranded.saturating_add((unsent - counted) as u32); + self.stranded = Some(Stranded { left: unsent, since: self.clock.nanos() }); + } + } } - let rearmed = causes & cause::LSC != 0 && self.unsent() > 0; - if rearmed { - self.rearm().map_err(PassRefused::Rearm)?; + if self.stranded.is_some() && self.link.is_up() { + self.reclaim_tx(); + } + if let (Some(Stranded { left, since }), true) = (self.stranded, self.link.is_up()) { + let waited = self.clock.nanos().saturating_sub(since); + if waited >= STRANDED_DEADLINE_NANOS { + return Err(PassRefused::Stranded { left, after_nanos: waited }); + } } Ok(Pass { messages, causes, link_changed: self.link != before, rearmed }) } + /// How long until a pass has to begin whether or not a message came: when + /// the descriptors a link change stranded are owed. `None` where nothing + /// is owed, which is every ring but one a link change found unsent frames + /// in, and that one too while its link is down. + pub fn pass_due_in(&self) -> Option { + let Stranded { since, .. } = self.stranded?; + let waited = self.clock.nanos().saturating_sub(since); + self.link.is_up().then(|| STRANDED_DEADLINE_NANOS.saturating_sub(waited)) + } + /// Transmit descriptors published and not written back, the written-back /// ones reclaimed first. fn unsent(&mut self) -> usize { @@ -1193,14 +1288,13 @@ impl I219 { (self.tx_next + TX_RING - self.tx_clean) % TX_RING } - /// Reset the function and bring it up again, as [`Self::open`] did, with + /// Reset the 82574 and bring it up again, as [`Self::open`] did, with /// both rings empty and the multicast table as it stood. - /// - /// §10.2.6.1 on `TCTL.EN`: "Software should combine this with a reset if - /// the packets in the FIFO need to be flushed" — and §10.2.6.7's head is - /// software's to write only after one. fn rearm(&mut self) -> Result<(), Refusal> { let unsent = self.unsent() as u32; + // The reset clears the statistics registers, so what they hold is + // taken first. + self.wire(); let dwords = match self.part { Part::E82574 => regs::MTA_DWORDS, Part::I219 => regs::MTA_DWORDS_PCH, @@ -1435,6 +1529,12 @@ impl I219 { self.dma.observe(); self.tx_clean = (self.tx_clean + 1) % TX_RING; self.counters.sent = self.counters.sent.saturating_add(1); + // Stranded descriptors are the ring's oldest, so each one taken + // back while any is owed is one of them. + self.stranded = match self.stranded { + Some(Stranded { left, since }) if left > 1 => Some(Stranded { left: left - 1, since }), + _ => None, + }; } } diff --git a/toyos-i219/src/stub.rs b/toyos-i219/src/stub.rs index 261ce135028..4882bcbfccd 100644 --- a/toyos-i219/src/stub.rs +++ b/toyos-i219/src/stub.rs @@ -95,9 +95,11 @@ pub struct Permits { /// §7.4.5: an interrupt whose cause was already cleared. "This results in /// a spurious interrupt." pub spurious_interrupts: bool, - /// No Intel document says what a part does with the transmit descriptors - /// it holds when its link goes away. On: it never sends them, and takes - /// no other, until it is reset. Off: it sends them when the link returns. + /// What a part does with the transmit descriptors it holds when its link + /// goes away. Off: it sends them when the link returns, which is §10.2.7's + /// Defer Count on the 82574 — a transmit deferred because "The link is not + /// up". On: it never sends them, and takes no other, until it is reset, + /// which no document rules out of the PCH's MAC. pub link_loss_strands_the_ring: bool, /// §10.2.4.1 case 3: "Interrupt was not asserted (ICR.INT_ASSERT=0): Read /// has no side affect." The document's own §7.4.5 says instead that "all @@ -2386,6 +2388,18 @@ impl Nic { self.0.borrow().messages } + /// The messages the function sent never reach the claim, and its causes + /// stay recorded: what a part that raises no message for an unmasked + /// cause looks like to a pass something else begins. + pub fn messages_are_lost(&self) { + self.0.borrow_mut().messages = 0; + } + + /// The clock moves on by `nanos` with nothing reaching the part. + pub fn time_passes(&self, nanos: u64) { + self.0.borrow_mut().nanos += nanos; + } + /// Stop the device acting on a tail register write, so it acts only when /// [`Self::run`] says so. pub fn hold(&self) { diff --git a/toyos-i219/src/tests.rs b/toyos-i219/src/tests.rs index b19e27ebd57..2ce7e86f598 100644 --- a/toyos-i219/src/tests.rs +++ b/toyos-i219/src/tests.rs @@ -723,7 +723,10 @@ fn the_transmit_cause_is_armed_only_when_asked() { assert_eq!(nic.peek(regs::IMS), unasked); assert_eq!(nic.pending(), 0, "{}", nic.because("a write-back nobody waited on sent a message")); settle(&nic, &mut driver); - one_pass(&mut driver); + // §7.4.5's spurious message over the cause nobody unmasked: no wake. + nic.spurious(); + let pass = one_pass(&mut driver); + assert!(pass.messages > 0 && pass.causes & tx_done == tx_done); assert_eq!(waits(&driver), (0, 0, 0), "{}", nic.because("a ring nobody waited on was counted")); // Asked, on a full ring. @@ -739,17 +742,48 @@ fn the_transmit_cause_is_armed_only_when_asked() { assert_eq!(waits(&driver), (1, 1, 0)); nic.run(); assert!(nic.pending() > 0, "{}", nic.because("a write-back the caller waited on sent no message")); + + // §7.4.3: "The cause bit stores the interrupt event regardless of the + // state of the mask bit" — so a pass something else began reads the + // cause of a part that raised no message for it, and that is no wake. + nic.messages_are_lost(); + let pass = one_pass(&mut driver); + assert_eq!(pass.causes & tx_done, tx_done); + assert_eq!((pass.messages, waits(&driver)), (0, (1, 1, 0)), "{}", nic.because("a cause no message came for was counted a wake")); + settle(&nic, &mut driver); + one_pass(&mut driver); + + // Nor is a message that came with a received frame's cause: it may + // have been that cause's. + fill(&nic, &mut driver, 0x50); + assert_eq!(driver.wake_on_room(), 0); + let inbound = frame(0x51, 80); + nic.deliver(&inbound); + nic.run(); + let pass = one_pass(&mut driver); + assert!(pass.messages > 0 && pass.causes & tx_done == tx_done && pass.causes & cause::RXT0 != 0); + assert_eq!(waits(&driver), (2, 2, 0), "{}", nic.because("a message another cause shared was counted a wake")); + assert_eq!(drain(&nic, &mut driver), vec![inbound]); + settle(&nic, &mut driver); + one_pass(&mut driver); + + // A message, and the transmit cause the only unmasked one read: taken. + fill(&nic, &mut driver, 0x60); + assert_eq!(driver.wake_on_room(), 0); + assert_eq!((nic.pending(), waits(&driver)), (0, (3, 3, 0))); + nic.run(); + assert!(nic.pending() > 0, "{}", nic.because("a write-back the caller waited on sent no message")); let pass = one_pass(&mut driver); assert_eq!(pass.causes & tx_done, tx_done); assert!(driver.tx_room() > 0, "{}", nic.because("the message came and the room did not")); - assert_eq!(waits(&driver), (1, 1, 1), "{}", nic.because("the wake was taken and not counted")); + assert_eq!(waits(&driver), (3, 3, 1), "{}", nic.because("the wake was taken and not counted")); // And not asked again: the pass that took the message masked the cause. assert_eq!(nic.peek(regs::IMS), unasked, "{}", nic.because("the pass left the transmit cause unmasked")); settle(&nic, &mut driver); assert_eq!(nic.pending(), 0, "{}", nic.because("a write-back after the wake was taken sent a message")); one_pass(&mut driver); - assert_eq!(waits(&driver), (1, 1, 1), "{}", nic.because("a cause nobody waited on was counted a wake")); + assert_eq!(waits(&driver), (3, 3, 1), "{}", nic.because("a cause nobody waited on was counted a wake")); } } @@ -814,24 +848,30 @@ fn a_link_that_is_down_has_no_room() { } } -/// What a part does with the descriptors it holds when its link goes away is -/// in no Intel document, so the driver asks nothing of it: the pass that reads -/// `LSC` over unsent descriptors resets the function and brings it up again -/// (§10.2.6.7: the head is software's to write only after a reset). The ring -/// is whole when the link returns, on a part that would never have sent those -/// frames and on one that would; the frames are counted; the multicast table -/// and the receive ring are as they were. +/// A frame the 82574 holds for a link that went away is not one for the link +/// that comes: the pass that reads `LSC` over unsent descriptors resets the +/// function and brings it up again (§10.2.6.7: the head is software's to write +/// only after a reset). The ring is whole when the link returns, on a part +/// that would never have sent those frames and on one that would; the frames +/// are counted, and so are the ones the statistics registers held; the +/// multicast table and the receive ring are as they were. #[test] fn a_ring_the_link_left_full_is_taken_back() { const MDNS: [u8; 6] = [0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb]; let mut reached_at_once = 0; - for ((part, _), strands, seen_down) in PARTS - .into_iter() - .flat_map(|part| [(part, true, true), (part, false, true), (part, true, false), (part, false, false)]) - { + for (strands, seen_down) in [(true, true), (false, true), (true, false), (false, false)] { let permits = Permits { link_loss_strands_the_ring: strands, ..Permits::default() }; - let (nic, mut driver) = held_with_a_link(26, part, permits); - let why = |what: &str| nic.because(&format!("{part:?}, strands {strands}, seen down {seen_down}: {what}")); + let (nic, mut driver) = held_with_a_link(26, Part::E82574, permits); + let why = |what: &str| nic.because(&format!("strands {strands}, seen down {seen_down}: {what}")); + // Three frames leave first, and nothing reads the statistics before + // the reset clears them. + for tag in 1..=3u8 { + let slot = driver.tx_reserve(100).expect("a ring with room"); + nic.put_bytes(slot.at, &frame(tag, 100)); + driver.tx_commit(slot); + } + settle(&nic, &mut driver); + assert_eq!(nic.sent().len(), 3); driver.accept_multicast(MDNS); let table: Vec = (0..regs::MTA_DWORDS).map(|dword| nic.peek(regs::MTA + dword * 4)).collect(); @@ -845,6 +885,7 @@ fn a_ring_the_link_left_full_is_taken_back() { let pass = one_pass(&mut driver); assert!(pass.rearmed, "{}", why("the ring was left with the link's unsent frames")); assert_eq!(driver.counters().unsent, (TX_RING - 1) as u32, "{}", why("the frames given back were not counted")); + assert_eq!(driver.wire().sent, 3, "{}", why("the reset took the frames the MAC had counted")); if seen_down { assert_eq!(driver.tx_room(), 0, "{}", why("a link that is down had room")); } @@ -888,6 +929,94 @@ fn a_ring_the_link_left_full_is_taken_back() { assert!(reached_at_once > 0, "no part had its link back at the end of the reset"); } +/// The PCH's MAC is not reset over a ring it holds descriptors in: no Intel +/// document says that is safe on it. The descriptors a link change found +/// unsent stay its own and are counted once; there is no room while the link +/// is down and nothing is owed; and a part that sends them when the link +/// returns goes on with a whole ring, however long after. +#[test] +fn the_pch_mac_keeps_the_ring_a_link_change_left_full() { + for seen_down in [true, false] { + let permits = Permits { link_loss_strands_the_ring: false, ..Permits::default() }; + let (nic, mut driver) = held_with_a_link(28, Part::I219, permits); + let why = |what: &str| nic.because(&format!("seen down {seen_down}: {what}")); + let phy_resets = nic.phy_resets(); + assert_eq!(driver.pass_due_in(), None); + + let out = fill(&nic, &mut driver, 1); + nic.set_link(false); + if seen_down { + let pass = one_pass(&mut driver); + assert!(!pass.rearmed, "{}", why("the function was reset over a ring it held descriptors in")); + assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32, "{}", why("the descriptors left were not counted")); + assert_eq!(driver.tx_room(), 0, "{}", why("a link that is down had room")); + assert_eq!(driver.pass_due_in(), None, "{}", why("a write-back was owed on a link that is down")); + } + nic.set_link(true); + nic.negotiation_settles(); + let pass = one_pass(&mut driver); + assert!(!pass.rearmed && driver.link().is_up()); + assert_eq!(nic.phy_resets(), phy_resets, "{}", why("the function was reset over a ring it held descriptors in")); + assert_eq!( + (driver.counters().stranded, driver.counters().unsent), + ((TX_RING - 1) as u32, 0), + "{}", + why("the descriptors left were not counted once") + ); + assert!(driver.pass_due_in().is_some_and(|due| due > 0), "{}", why("nothing was owed for the descriptors left")); + + // Written back with nobody asking for room: the pass that is due finds + // them itself. + for _ in 0..TX_RING * 4 { + nic.run(); + } + assert_eq!(nic.sent(), out, "{}", why("what left when the link returned is not what the ring held")); + nic.time_passes(STRANDED_DEADLINE_NANOS * 2); + let pass = driver.begin_pass().unwrap_or_else(|refused| panic!("{}", why(&format!("a ring written back whole: {refused}")))); + assert!(!pass.rearmed); + assert_eq!(driver.pass_due_in(), None, "{}", why("a write-back was owed on a ring written back whole")); + assert_eq!(driver.tx_room(), TX_RING - 1); + } +} + +/// A PCH MAC that has not written those descriptors back by the time the +/// slowest link the documents allow would have emptied the ring is handed up +/// by name — measured from the last link change read over them, and never +/// while the link is down. +#[test] +fn a_ring_the_pch_mac_never_writes_back_is_refused() { + let (nic, mut driver) = held_with_a_link(29, Part::I219, Permits::default()); + fill(&nic, &mut driver, 1); + nic.set_link(false); + one_pass(&mut driver); + nic.time_passes(STRANDED_DEADLINE_NANOS * 2); + one_pass(&mut driver); + link_up(&nic, &mut driver); + + // A second change over the same descriptors counts none of them again and + // is where the deadline runs from. + nic.time_passes(STRANDED_DEADLINE_NANOS / 2); + nic.set_link(false); + nic.set_link(true); + nic.negotiation_settles(); + one_pass(&mut driver); + assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32); + nic.time_passes(STRANDED_DEADLINE_NANOS * 3 / 4); + nic.run(); + one_pass(&mut driver); + assert_eq!(driver.tx_room(), 0, "{}", nic.because("a ring the part never wrote back had room")); + + let due = driver.pass_due_in().expect("a write-back is owed"); + assert!(due > 0 && due < STRANDED_DEADLINE_NANOS / 4); + nic.time_passes(due); + assert_eq!(driver.pass_due_in(), Some(0)); + let Err(PassRefused::Stranded { left, after_nanos }) = driver.begin_pass() else { + panic!("{}", nic.because("a ring the part never wrote back began a pass past its deadline")); + }; + assert_eq!(left, TX_RING - 1); + assert!(after_nanos >= STRANDED_DEADLINE_NANOS); +} + /// A function that does not come back from that reset is handed up by name, /// and not driven on. #[test] diff --git a/userland/netstack/src/card.rs b/userland/netstack/src/card.rs index 88e5d1e5307..b74b2926503 100644 --- a/userland/netstack/src/card.rs +++ b/userland/netstack/src/card.rs @@ -63,8 +63,10 @@ impl Card { /// silently never arrives, so this dies where it can be read — once, for /// whichever driver is running. /// - /// So does an Intel part that did not come back from the reset a link - /// change over unsent frames costs it: nothing drives it from there. + /// So does an 82574 that did not come back from the reset a link change + /// over unsent frames costs it, and an I219 that kept the frames such a + /// change left in its ring past the driver's deadline: nothing drives + /// either from there. /// /// Answers the link where the pass found it changed; virtio reports none. pub fn begin_pass(&self) -> Option { @@ -121,6 +123,7 @@ impl Card { snap.put("descriptors.sent", counters.sent); snap.put("descriptors.received", counters.received); snap.put("descriptors.unsent", counters.unsent); + snap.put("descriptors.stranded", counters.stranded); snap.put("transmit.full", counters.tx_full); snap.put("transmit.wake_armed", counters.tx_wake_armed); snap.put("transmit.wake_taken", counters.tx_wake_taken); @@ -160,6 +163,16 @@ impl Card { } } + /// How long until a pass has to begin whether or not the claim reads + /// ready, in nanoseconds: [`Self::begin_pass`] is where a card that owes + /// something by then is refused. virtio owes nothing. + pub fn pass_due_in(&self) -> Option { + match self { + Self::Virtio(_) => None, + Self::Intel(nic) => nic.pass_due_in(), + } + } + /// Say what the driver counted, once a pass and after every frame the pass /// sent: a line per refused descriptor is itself more frames to send. pub fn report(&self) { diff --git a/userland/netstack/src/i219.rs b/userland/netstack/src/i219.rs index e051e95cf23..3f35a4d0048 100644 --- a/userland/netstack/src/i219.rs +++ b/userland/netstack/src/i219.rs @@ -293,7 +293,7 @@ impl Nic { /// Take the interrupt, acknowledge its causes and refresh the link — and /// answer the link where this pass found it changed. /// - /// A pass that reset the function to take its transmit ring back says so, + /// A pass that reset the 82574 to take its transmit ring back says so, /// with what the bring-up found the second time. pub fn begin_pass(&self) -> Result, toyos_i219::PassRefused> { let pass = self.driver.borrow_mut().begin_pass()?; @@ -307,6 +307,12 @@ impl Nic { Ok(pass.link_changed.then(|| self.link())) } + /// How long until a pass has to begin with nothing arrived: the driver's + /// deadline for descriptors a link change left in the ring. + pub fn pass_due_in(&self) -> Option { + self.driver.borrow().pass_due_in() + } + pub fn poll_rx(&self) -> Option { self.driver.borrow_mut().poll_rx() } @@ -390,7 +396,8 @@ impl Nic { crate::say!( "netstack: I219: refused {} over-length, {} errored, {} split and {} empty \ descriptor(s); {} overrun(s), {} descriptor-starvation report(s), \ - {} frame(s) given back unsent at a link change, {} spurious interrupt(s)", + {} frame(s) given back unsent and {} left in the ring at a link change, \ + {} spurious interrupt(s)", counters.over_length, counters.errored, counters.split, @@ -398,6 +405,7 @@ impl Nic { counters.overruns, counters.starved, counters.unsent, + counters.stranded, counters.spurious, ); } diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index 7a0afafdde4..76e8c7fcc90 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -1574,6 +1574,8 @@ fn main() { Some(left) => timeout.min(left.as_nanos() as u64), None => timeout, }; + // A card that never does what it owes sends no interrupt to say so. + let timeout = timeout.min(device.nic.pass_due_in().unwrap_or(u64::MAX)); // A client that connects and then says nothing wakes nothing, so the // deadline that removes it has to be a wake in its own right: without // this netstack can sit in `wait` forever with `pending` full of clients From 9c3b815387187e2bc2b6a6f67d8d67aae21fee12 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 01:07:53 +0200 Subject: [PATCH 5/7] One rule for both Intel parts at a link change: the descriptors stay the part's, and the re-arm is gone The 82574 datasheet's Defer Count counts a transmit deferred because "The link is not up", so the path built for the I219 serves both parts: no room while the link is down, the descriptors a link change finds unsent counted and left in the ring, their write-back owed within one bound from the pass that reads the link up, and a part that has not written them back refused by name. `rearm`, the split of the bring-up it needed, `Pass::rearmed`, `PassRefused::Rearm`, `Counters::unsent` and `descriptors.unsent` are deleted, with the two tests and the stub modelling only they used; `open` is main's single bring-up again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...-read-an-intel-nic-across-a-link-change.md | 50 +- ...cs-room-and-wake-are-unread-on-hardware.md | 2 +- toyos-i219/src/lib.rs | 535 ++++++++---------- toyos-i219/src/stub.rs | 14 +- toyos-i219/src/tests.rs | 242 +++----- userland/netstack/src/card.rs | 7 +- userland/netstack/src/i219.rs | 14 +- 7 files changed, 321 insertions(+), 543 deletions(-) diff --git a/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md index d8297eb1eaf..303705d471c 100644 --- a/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md +++ b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md @@ -6,35 +6,35 @@ opened: 2026-10-09 # No machine has read an Intel NIC across a link change, and two things about the I219 there are unknown -A link that changes while the transmit ring holds unsent descriptors is -answered per part (`toyos-i219/src/lib.rs`, "A frame is offered only to a ring -with room for it"), and every reading of either answer is the driver's own +A link that changes while the transmit ring holds unsent descriptors leaves +them the part's, on the 82574 and on the I219 alike +(`toyos-i219/src/lib.rs`, "A frame is offered only to a ring with room for +it"): they are counted `descriptors.stranded`, the part is owed their +write-back within `STRANDED_DEADLINE_NANOS` (12.98 s, the slowest the 82574 +datasheet's §10.2.6.1 lets a full ring leave) of the link being read up, and a +part that has not written them back by then is refused by name, so netstack +ends with "cannot be driven on". Every reading of that is the driver's own model. -**The 82574** is reset and brought up again, by its own document: §10.2.6.1 -("Software should combine this with a reset if the packets in the FIFO need to -be flushed") and §10.2.6.7. The same document's Defer Count (§10.2.7) says the -part would otherwise have kept those frames for the link that returns. No -82574 is in reach, and QEMU's `e1000e` is not in the harness. +**The 82574's document says it writes them back**: its Defer Count (§10.2.7) +counts a transmit deferred because "The link is not up". No 82574 is in +reach, and QEMU's `e1000e` is not in the harness. -**The I219 is not reset over a ring it holds descriptors in**, because two -things are unknown of it: +**Of the I219 two things are unknown:** - **Whether it writes those descriptors back when the link returns.** The - 82574's Defer Count is the only sentence on the subject in any document - read, and it is the other part's. The driver gives the I219 the slowest - time that document lets a full ring leave (`STRANDED_DEADLINE_NANOS`, - 12.98 s) from the link being read up, and then refuses it by name: netstack - ends with "cannot be driven on". + Defer Count is the only sentence on the subject in any document read, and + it is the other part's. A part that does not is refused at the deadline. - **Whether a reset over a live ring hangs it.** Nothing Intel publishes that - was found says it is safe or that it is not. Read, and silent on it: the - I219 datasheet (612523 rev 2.02), the 500 Series on-package PCH datasheet - volumes 1 and 2 (631119-007 §18, 631120-002 §8), the 100 Series PCH - datasheet volume 2 (332691) and specification update (332692-015), the 400 - Series PCH datasheet volume 2 (620855-002) and specification update - (620856-009), the 300 Series PCH specification update (337349-012), the 7th - generation U/Y platform I/O specification update (334660-009), the I218 - specification update (rev 1.0), and the 82574 datasheet (317694 rev 3.4) and + was found says it is safe or that it is not, which is why neither part is + reset to take a ring back. Read, and silent on it: the I219 datasheet + (612523 rev 2.02), the 500 Series on-package PCH datasheet volumes 1 and 2 + (631119-007 §18, 631120-002 §8), the 100 Series PCH datasheet volume 2 + (332691) and specification update (332692-015), the 400 Series PCH + datasheet volume 2 (620855-002) and specification update (620856-009), the + 300 Series PCH specification update (337349-012), the 7th generation U/Y + platform I/O specification update (334660-009), the I218 specification + update (rev 1.0), and the 82574 datasheet (317694 rev 3.4) and specification update. intel.com's search finds no I219 specification update, sighting or application note, and no specification update for the 500 Series on-package PCH. The one piece of evidence the tree has is @@ -65,5 +65,5 @@ away). The reset over a live ring is read by a row that starts netstack again under traffic and reboots: the machine comes back, or it does not. An Intel document that states the I219's sequence for a reset over published -descriptors closes the second unknown without a machine: the driver then -does exactly that, in `open` as well. +descriptors closes the second unknown without a machine: `open` then does +exactly that. diff --git a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md index 2f2847a279f..abac3c28a82 100644 --- a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md +++ b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md @@ -46,7 +46,7 @@ A T14 boot whose netstack drives `8086:15fc` queues more than `toyos_i219::TX_RING - 1` frames in one pass, from more sockets than one and to a destination that answers none of them, and `inspect` then answers `transmit.full`, `transmit.wake_armed` and `transmit.wake_taken` each at least -1, `descriptors.unsent` and `descriptors.stranded` 0, and `descriptors.sent` +1, `descriptors.stranded` 0, and `descriptors.sent` equal to `wire.sent`; the boot's log carries no "cannot be driven on" line from netstack. A boot whose `transmit.full` reads 0 never filled the ring and has read nothing. diff --git a/toyos-i219/src/lib.rs b/toyos-i219/src/lib.rs index 525d218ca37..1d20641aad2 100644 --- a/toyos-i219/src/lib.rs +++ b/toyos-i219/src/lib.rs @@ -39,24 +39,18 @@ //! of a descriptor, and nothing waits on the part. //! //! **A link that is down has no room**, and no frame is published while it -//! is. What the 82574 does with one it already holds is in §10.2.7's Defer -//! Count: "A defer event occurs when the transmitter cannot immediately send a -//! packet due to the medium being busy either because: [...] The link is not -//! up" — it keeps the frame for the link that comes. A pass that reads `LSC` -//! with a descriptor still unsent ([`I219::begin_pass`]) answers per part: -//! -//! - **The 82574 gives the ring back**, because a frame queued for a link that -//! has gone is not one for the link that came after it: the function is -//! reset and brought up again. §10.2.6.1 on `TCTL.EN`: "Software should -//! combine this with a reset if the packets in the FIFO need to be flushed", -//! and §10.2.6.7 lets software write the ring's head only "after a reset -//! (hardware reset or CTRL.RST) and before enabling the transmit function". -//! - **The PCH's MAC is never reset over a ring it holds descriptors in.** No -//! Intel document says that is safe on that part and none says what it does -//! with those descriptors, so they stay its own: it is owed their write-back -//! within `STRANDED_DEADLINE_NANOS` of the link being read up over them, the -//! ring goes on when they come, and a part that has not written them back by -//! then is refused by name ([`PassRefused::Stranded`]) and driven no further. +//! is. One the part already holds it keeps for the link that comes: §10.2.7's +//! Defer Count has "A defer event occurs when the transmitter cannot +//! immediately send a packet due to the medium being busy either because: +//! [...] The link is not up". That sentence is the 82574's, and the PCH's MAC +//! has none of its own. So the descriptors a link change finds unsent stay the +//! part's, on both: it is owed their write-back within +//! `STRANDED_DEADLINE_NANOS` of the link being read up over them +//! ([`I219::begin_pass`]), the ring goes on when they come, and a part that +//! has not written them back by then is refused by name +//! ([`PassRefused::Stranded`]) and driven no further. **Neither is reset to +//! take the ring back**: no Intel document says a reset over published +//! descriptors is safe on the PCH's MAC, and the 82574 has no need of one. //! //! # The device is not trusted //! @@ -408,9 +402,6 @@ pub struct Pass { pub causes: u32, /// Whether the link's state is not what it was at the last pass. pub link_changed: bool, - /// Whether this pass reset the function and brought it up again, to take - /// back a transmit ring a link change left with unsent descriptors. - pub rearmed: bool, } /// Why [`I219::begin_pass`] did not begin a pass. @@ -419,10 +410,7 @@ pub enum PassRefused { /// The claim refused its interrupt record, in its own word: the function /// is no longer this driver's. Claim(C), - /// The function did not come back up from the reset that takes its - /// transmit ring back. - Rearm(Refusal), - /// The PCH's MAC still holds `left` of the descriptors a link change left + /// The part still holds `left` of the descriptors a link change left /// in its transmit ring, this long after the link was read up over them. Stranded { left: usize, after_nanos: u64 }, } @@ -431,10 +419,6 @@ impl core::fmt::Display for PassRefused { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { Self::Claim(why) => write!(f, "its claim refused an interrupt read: {why:?}"), - Self::Rearm(why) => write!( - f, - "it did not come back from the reset that takes its transmit ring back: {why}" - ), Self::Stranded { left, after_nanos } => write!( f, "a link change left its transmit ring holding frames, and {} ms after the link \ @@ -471,11 +455,8 @@ pub struct Counters { /// while it was unmasked: the wake a full ring was waiting on, taken, and /// nothing a pass begun by anything else reads. pub tx_wake_taken: u32, - /// Frames published and never sent: the descriptors the 82574's reset - /// gave back at a link change ([`I219::begin_pass`]). - pub unsent: u32, - /// Descriptors a link change found unsent in the PCH MAC's ring, and left - /// there for it to write back. + /// Descriptors a link change found unsent in the ring, and left there for + /// the part to write back ([`I219::begin_pass`]). pub stranded: u32, /// Messages that arrived with no cause set in `ICR` — §7.4.5's spurious /// interrupt. It costs a pass and nothing else. @@ -547,8 +528,12 @@ const RESET_SETTLE_NANOS: u64 = 1_000; /// the PCIe Master Enable Status bit is not cleared within a given time". const MASTER_QUIESCE_DEADLINE_NANOS: u64 = 10_000_000; -/// How long the PCH's MAC has to write back the descriptors a link change left -/// in its transmit ring, from the pass that read the link up over them. +/// How long a part has to write back the descriptors a link change left in its +/// transmit ring, from the pass that read the link up over them. +/// +/// **One bound for both parts**, because both are driven through the one +/// `TCTL` this driver writes and §10.2.6.1 is the only document of it: the +/// PCH's MAC publishes no transmit timing of its own (631120 §8.2). /// /// **A ceiling on an event, and the slowest §10.2.6.1 lets a full ring leave**: /// [`TX_RING`]` - 1` frames of a whole buffer each, on a half-duplex link at @@ -823,8 +808,8 @@ pub struct I219 { /// Whether [`Part::tx_done`] is unmasked: a caller found the ring full /// and no pass has begun since. tx_wake: bool, - /// The descriptors a link change left in the PCH MAC's transmit ring that - /// it has not yet written back. + /// The descriptors a link change left in the transmit ring that the part + /// has not yet written back. stranded: Option, counters: Counters, /// What [`Self::wire`] has read out of the statistics registers so far. @@ -849,216 +834,6 @@ struct Stranded { since: u64, } -/// What [`program`] leaves beside the registers it wrote. -struct Programmed { - mac: [u8; 6], - brought_up: BringUp, -} - -/// §4.6's bring-up from the reset to the interrupt mask: everything -/// [`I219::open`] programs, which is also everything a reset takes away. -fn program( - part: Part, - regs: &R, - clock: &C, - dma: &D, -) -> Result { - // The PHY is brought within reach before the reset, and the reset - // takes it with the MAC: [`wake`]'s header is why, on the part whose - // PHY is not on the MAC's own die. - let (woke, whole) = match part { - Part::I219 => { - let woke = phy::wake(regs, clock, None); - (Some(woke), Whole::WithThePhy { after: Some(clock.nanos()) }) - } - Part::E82574 => (None, Whole::MacAlone), - }; - let Reset { master_quiet, at: reset_at, full, released } = - reset(regs, clock, whole)?; - - // §10.2.5.23: the reset reloads entry 0 from the NVM, so this is read - // after it and not before. - let low = regs.read(regs::RAL0); - let high = regs.read(regs::RAH0); - if high & rah::AV == 0 { - return Err(Refusal::NoStationAddress); - } - let mac = [ - low as u8, - (low >> 8) as u8, - (low >> 16) as u8, - (low >> 24) as u8, - high as u8, - (high >> 8) as u8, - ]; - - // What the PCH's MAC is given after its reset and before its rings: - // [`pch`]'s header. After the last refusal above, so the one place a - // refusal has the word to give back is below. - if part == Part::I219 { - pch::prepare(regs); - } - - // §4.6.5: "Set up the Multicast Table Array (MTA) per software. This - // generally means zeroing all entries initially." The table this part - // has, and nothing past it. - let table = match part { - Part::E82574 => regs::MTA_DWORDS, - Part::I219 => regs::MTA_DWORDS_PCH, - }; - for entry in 0..table { - regs.write(regs::MTA + entry * 4, 0); - } - - // §4.6.3.1: "Refer to the PHY documentation for the initialization and - // link setup steps. The device driver uses the MDIC register to - // initialize the PHY and setup the link." The PHY documentation this - // driver has is the I219's, and §10.2.2.7 addresses the 82574's own PHY - // under a scheme of its own — so the sequence is refused by name on the - // part it was not written from. - let phy = match part { - Part::I219 => phy::bring_up(regs, clock, reset_at, released), - Part::E82574 => Err(phy::PhyRefusal::NotThisRegisterMap), - }; - - // §10.2.2.1: `SLU` is what lets the MAC see the PHY's link at all; - // `ASDE` must be zero on this family; forcing speed or duplex would - // override what auto-negotiation resolved; and this driver negotiates - // no flow control and strips no VLAN tag. §3.1.3.10's master disable - // goes with them and is not preserved: a part that came out of the - // reset still blocking master requests would fetch no descriptor and - // write back no frame, and nothing else in this bring-up would say so. - let held = regs.read(regs::CTRL); - let wanted = (held - & !(ctrl::GIO_MASTER_DISABLE - | ctrl::PHY_RST - | ctrl::ASDE - | ctrl::ILOS - | ctrl::FRCSPD - | ctrl::FRCDPLX - | ctrl::RFCE - | ctrl::TFCE - | ctrl::VME)) - | ctrl::SLU; - regs.write(regs::CTRL, wanted); - - // §10.2.4.7: "If any bits are set in EIAC, the ICR register should not - // be read" — and this driver reads it, so auto-clear stays off and - // every cause is acknowledged by the write-back in `begin_pass`. - regs.write(regs::EIAC, 0); - - // A refusal from here on gives the word to the firmware back: nothing - // holds the function that a drop would let go. - let armed = rings_and_mask(regs, dma); - if armed.is_err() && part == Part::I219 { - pch::release(regs); - } - armed?; - Ok(Programmed { mac, brought_up: BringUp { master_quiet, woke, reset: full, phy } }) -} - -/// The rest of §4.6: the vector allocation, both rings, the transmitter, the -/// receiver, and the interrupt mask last. -fn rings_and_mask(regs: &R, dma: &D) -> Result<(), Refusal> { - - // §10.2.4.9: `IVAR` allocates every cause to no vector at reset, so a - // part in MSI-X mode with it unprogrammed fills `ICR` and delivers - // nothing. Read back, because the document defines the register only - // "in MSI-X mode" and says nothing about what a part outside that mode - // answers — so a part that does not take the write is refused here - // rather than driven on a guess about which interrupt it would raise. - regs.write(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); - accepted(regs, regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; - - // No moderation on either side: §10.2.4.2's throttle and the two - // receive timers all hold an interrupt back, and what this driver - // waits on is the frame that has already arrived. - regs.write(regs::ITR, 0); - regs.write(regs::RDTR, 0); - regs.write(regs::RADV, 0); - regs.write(regs::TIDV, 0); - regs.write(regs::TADV, 0); - - arm_rx_ring(regs, dma); - arm_tx_ring(regs, dma); - - // §4.6.6, in its order: the write-back policy, the gap, then the - // transmitter. - regs.write(regs::TXDCTL, txdctl::SUGGESTED); - regs.write(regs::TIPG, regs::TIPG_DEFAULT); - regs.write(regs::TCTL, TX_CONTROL); - accepted(regs, regs::TCTL, TX_CONTROL)?; - - // §4.6.5.1: the receiver last, "only after all other setup is - // accomplished". - let rx = rctl::EN | rctl::BAM | rctl::SECRC | rctl::BSIZE_2048; - regs.write(regs::RCTL, rx); - accepted(regs, regs::RCTL, rx)?; - - // §4.6.5: and only now the mask, so no cause can arrive before there - // is a ring to answer it with. - regs.write(regs::IMS, cause::ENABLED | cause::ENABLED_MSIX); - Ok(()) -} - -/// A register wrote what it was told, so a window that is not this register -/// file is refused here instead of looking like a dead network. -fn accepted(regs: &R, reg: usize, wrote: u32) -> Result<(), Refusal> { - let read = regs.read(reg); - if read & wrote == wrote { - Ok(()) - } else { - Err(Refusal::NotAccepted { reg, wrote, read }) - } -} - -/// Publish every receive descriptor and hand the ring to the device. -/// -/// §4.6.5.1: base, length, head, then "the tail pointer should be set to -/// point one descriptor beyond the end" — so the ring holds `RX_RING` -/// buffers and `RX_RING - 1` of them are the device's at any moment, the -/// last being the one §7.1.8's "head equals tail is empty" costs. -fn arm_rx_ring(regs: &R, dma: &D) { - for index in 0..RX_RING { - publish_rx(dma, index); - } - let base = dma.device_addr(OFF_RX_RING); - dma.publish(); - regs.write(regs::RDBAL, base as u32); - regs.write(regs::RDBAH, (base >> 32) as u32); - regs.write(regs::RDLEN, (RX_RING * rx_desc::BYTES) as u32); - regs.write(regs::RDH, 0); - regs.write(regs::RDT, (RX_RING - 1) as u32); -} - -fn arm_tx_ring(regs: &R, dma: &D) { - for index in 0..TX_RING { - let at = OFF_TX_RING + index * tx_desc::BYTES; - dma.write(at, 0); - dma.write(at + 8, 0); - } - let base = dma.device_addr(OFF_TX_RING); - dma.publish(); - regs.write(regs::TDBAL, base as u32); - regs.write(regs::TDBAH, (base >> 32) as u32); - regs.write(regs::TDLEN, (TX_RING * tx_desc::BYTES) as u32); - regs.write(regs::TDH, 0); - regs.write(regs::TDT, 0); -} - -/// Write descriptor `index` back as an empty buffer the device may fill. -/// -/// The status word is zeroed before the address is published, because -/// §7.1.8 says software can "zero the status byte in the descriptor to -/// make it ready for reuse" and a stale `DD` left behind would be read as -/// a completion for a frame that never arrived. -fn publish_rx(dma: &D, index: usize) { - let at = OFF_RX_RING + index * rx_desc::BYTES; - let buffer = OFF_RX_BUFS + index * RX_BUF_BYTES; - dma.write(at + 8, 0); - dma.write(at, dma.device_addr(buffer)); -} - impl I219 { /// Bring the function up in the order §4.6 fixes: reset with every /// interrupt masked, the station address, the multicast table, the PHY, the @@ -1077,7 +852,90 @@ impl I219 { if (dma.bytes() as u64) < GRANT_BYTES { return Err(Refusal::Grant { given: dma.bytes(), needed: GRANT_BYTES as usize }); } - let Programmed { mac, brought_up } = program(part, ®s, &clock, &dma)?; + // The PHY is brought within reach before the reset, and the reset + // takes it with the MAC: [`wake`]'s header is why, on the part whose + // PHY is not on the MAC's own die. + let (woke, whole) = match part { + Part::I219 => { + let woke = phy::wake(®s, &clock, None); + (Some(woke), Whole::WithThePhy { after: Some(clock.nanos()) }) + } + Part::E82574 => (None, Whole::MacAlone), + }; + let Reset { master_quiet, at: reset_at, full, released } = + reset(®s, &clock, whole)?; + + // §10.2.5.23: the reset reloads entry 0 from the NVM, so this is read + // after it and not before. + let low = regs.read(regs::RAL0); + let high = regs.read(regs::RAH0); + if high & rah::AV == 0 { + return Err(Refusal::NoStationAddress); + } + let mac = [ + low as u8, + (low >> 8) as u8, + (low >> 16) as u8, + (low >> 24) as u8, + high as u8, + (high >> 8) as u8, + ]; + + // What the PCH's MAC is given after its reset and before its rings: + // [`pch`]'s header. After the last refusal that comes before `Self` + // exists, so every refusal after it is a drop that lets the function go. + if part == Part::I219 { + pch::prepare(®s); + } + + // §4.6.5: "Set up the Multicast Table Array (MTA) per software. This + // generally means zeroing all entries initially." The table this part + // has, and nothing past it. + let table = match part { + Part::E82574 => regs::MTA_DWORDS, + Part::I219 => regs::MTA_DWORDS_PCH, + }; + for entry in 0..table { + regs.write(regs::MTA + entry * 4, 0); + } + + // §4.6.3.1: "Refer to the PHY documentation for the initialization and + // link setup steps. The device driver uses the MDIC register to + // initialize the PHY and setup the link." The PHY documentation this + // driver has is the I219's, and §10.2.2.7 addresses the 82574's own PHY + // under a scheme of its own — so the sequence is refused by name on the + // part it was not written from. + let phy = match part { + Part::I219 => phy::bring_up(®s, &clock, reset_at, released), + Part::E82574 => Err(phy::PhyRefusal::NotThisRegisterMap), + }; + + // §10.2.2.1: `SLU` is what lets the MAC see the PHY's link at all; + // `ASDE` must be zero on this family; forcing speed or duplex would + // override what auto-negotiation resolved; and this driver negotiates + // no flow control and strips no VLAN tag. §3.1.3.10's master disable + // goes with them and is not preserved: a part that came out of the + // reset still blocking master requests would fetch no descriptor and + // write back no frame, and nothing else in this bring-up would say so. + let held = regs.read(regs::CTRL); + let wanted = (held + & !(ctrl::GIO_MASTER_DISABLE + | ctrl::PHY_RST + | ctrl::ASDE + | ctrl::ILOS + | ctrl::FRCSPD + | ctrl::FRCDPLX + | ctrl::RFCE + | ctrl::TFCE + | ctrl::VME)) + | ctrl::SLU; + regs.write(regs::CTRL, wanted); + + // §10.2.4.7: "If any bits are set in EIAC, the ICR register should not + // be read" — and this driver reads it, so auto-clear stays off and + // every cause is acknowledged by the write-back in `begin_pass`. + regs.write(regs::EIAC, 0); + let mut nic = Self { part, regs, @@ -1085,7 +943,7 @@ impl I219 { dma, irq, mac, - brought_up, + brought_up: BringUp { master_quiet, woke, reset: full, phy }, link: Link::default(), opened_at: 0, link_up_at: None, @@ -1099,12 +957,50 @@ impl I219 { counters: Counters::default(), wire: Wire::default(), }; + + // §10.2.4.9: `IVAR` allocates every cause to no vector at reset, so a + // part in MSI-X mode with it unprogrammed fills `ICR` and delivers + // nothing. Read back, because the document defines the register only + // "in MSI-X mode" and says nothing about what a part outside that mode + // answers — so a part that does not take the write is refused here + // rather than driven on a guess about which interrupt it would raise. + nic.regs.write(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); + nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; + + // No moderation on either side: §10.2.4.2's throttle and the two + // receive timers all hold an interrupt back, and what this driver + // waits on is the frame that has already arrived. + nic.regs.write(regs::ITR, 0); + nic.regs.write(regs::RDTR, 0); + nic.regs.write(regs::RADV, 0); + nic.regs.write(regs::TIDV, 0); + nic.regs.write(regs::TADV, 0); + + nic.arm_rx_ring(); + nic.arm_tx_ring(); + + // §4.6.6, in its order: the write-back policy, the gap, then the + // transmitter. + nic.regs.write(regs::TXDCTL, txdctl::SUGGESTED); + nic.regs.write(regs::TIPG, regs::TIPG_DEFAULT); + nic.regs.write(regs::TCTL, TX_CONTROL); + nic.accepted(regs::TCTL, TX_CONTROL)?; + + // §4.6.5.1: the receiver last, "only after all other setup is + // accomplished". + let rx = rctl::EN | rctl::BAM | rctl::SECRC | rctl::BSIZE_2048; + nic.regs.write(regs::RCTL, rx); + nic.accepted(regs::RCTL, rx)?; + + // §4.6.5: and only now the mask, so no cause can arrive before there + // is a ring to answer it with. + nic.regs.write(regs::IMS, cause::ENABLED | cause::ENABLED_MSIX); + nic.opened_at = nic.clock.nanos(); nic.refresh_link(); Ok(nic) } - /// Pass frames sent to the multicast address `group`: the one bit of the /// Multicast Table Array its hash names on this part ([`regs::mta_bit_82574`], /// [`regs::mta_bit_pch`]) is set, and @@ -1118,6 +1014,64 @@ impl I219 { self.regs.write(at, self.regs.read(at) | 1 << bit); } + /// A register wrote what it was told, so a window that is not this register + /// file is refused here instead of looking like a dead network. + fn accepted(&self, reg: usize, wrote: u32) -> Result<(), Refusal> { + let read = self.regs.read(reg); + if read & wrote == wrote { + Ok(()) + } else { + Err(Refusal::NotAccepted { reg, wrote, read }) + } + } + + /// Publish every receive descriptor and hand the ring to the device. + /// + /// §4.6.5.1: base, length, head, then "the tail pointer should be set to + /// point one descriptor beyond the end" — so the ring holds `RX_RING` + /// buffers and `RX_RING - 1` of them are the device's at any moment, the + /// last being the one §7.1.8's "head equals tail is empty" costs. + fn arm_rx_ring(&mut self) { + for index in 0..RX_RING { + self.publish_rx(index); + } + let base = self.dma.device_addr(OFF_RX_RING); + self.dma.publish(); + self.regs.write(regs::RDBAL, base as u32); + self.regs.write(regs::RDBAH, (base >> 32) as u32); + self.regs.write(regs::RDLEN, (RX_RING * rx_desc::BYTES) as u32); + self.regs.write(regs::RDH, 0); + self.regs.write(regs::RDT, self.rx_tail as u32); + } + + fn arm_tx_ring(&mut self) { + for index in 0..TX_RING { + let at = OFF_TX_RING + index * tx_desc::BYTES; + self.dma.write(at, 0); + self.dma.write(at + 8, 0); + } + let base = self.dma.device_addr(OFF_TX_RING); + self.dma.publish(); + self.regs.write(regs::TDBAL, base as u32); + self.regs.write(regs::TDBAH, (base >> 32) as u32); + self.regs.write(regs::TDLEN, (TX_RING * tx_desc::BYTES) as u32); + self.regs.write(regs::TDH, 0); + self.regs.write(regs::TDT, 0); + } + + /// Write descriptor `index` back as an empty buffer the device may fill. + /// + /// The status word is zeroed before the address is published, because + /// §7.1.8 says software can "zero the status byte in the descriptor to + /// make it ready for reuse" and a stale `DD` left behind would be read as + /// a completion for a frame that never arrived. + fn publish_rx(&mut self, index: usize) { + let at = OFF_RX_RING + index * rx_desc::BYTES; + let buffer = OFF_RX_BUFS + index * RX_BUF_BYTES; + self.dma.write(at + 8, 0); + self.dma.write(at, self.dma.device_addr(buffer)); + } + pub fn mac(&self) -> [u8; 6] { self.mac } @@ -1175,18 +1129,13 @@ impl I219 { /// function is no longer this driver's, and the refusal is handed up rather /// than counted as quiet. /// - /// **On the 82574 a pass that reads `LSC` with a transmit descriptor still - /// unsent takes the ring back**, by the reset and the bring-up - /// [`Self::open`] runs: the frames in it are counted [`Counters::unsent`], - /// a frame received and not yet handed up goes with them, and the pass - /// lasts as long as a bring-up does. Every [`Frame`] is given back before - /// a pass begins. `LSC` says the link changed and not that it is down, so - /// a link that reads up on both sides of it costs the same. - /// - /// **On the PCH's MAC those descriptors stay the part's**, counted - /// [`Counters::stranded`], and a pass that finds one still not written - /// back `STRANDED_DEADLINE_NANOS` after the link was read up over them - /// is refused. [`Self::pass_due_in`] is when that pass has to begin. + /// **A pass that reads `LSC` with a transmit descriptor still unsent leaves + /// it the part's**, counted [`Counters::stranded`], and a pass that finds + /// one still not written back `STRANDED_DEADLINE_NANOS` after the link + /// was read up over them is refused. [`Self::pass_due_in`] is when that + /// pass has to begin. `LSC` says the link changed and not that it is + /// down, so a link that reads up on both sides of it starts the same + /// deadline. pub fn begin_pass(&mut self) -> Result> { self.rx_budget = RX_BUDGET; let messages = match self.irq.taken() { @@ -1240,23 +1189,16 @@ impl I219 { // On `LSC` alone: §7.4.3 records it masked or not, so a link that came // up before the mask was written is in this read all the same. let before = self.link; - let mut rearmed = false; if causes & cause::LSC != 0 { self.refresh_link(); - match (self.unsent(), self.part) { - (0, _) => {} - (_, Part::E82574) => { - self.rearm().map_err(PassRefused::Rearm)?; - rearmed = true; - } - (unsent, Part::I219) => { - // A second change over the same descriptors counts none of - // them twice, and the deadline runs from the newest. - let counted = self.stranded.map_or(0, |stranded| stranded.left); - self.counters.stranded = - self.counters.stranded.saturating_add((unsent - counted) as u32); - self.stranded = Some(Stranded { left: unsent, since: self.clock.nanos() }); - } + let unsent = self.unsent(); + if unsent > 0 { + // A second change over the same descriptors counts none of + // them twice, and the deadline runs from the newest. + let counted = self.stranded.map_or(0, |stranded| stranded.left); + self.counters.stranded = + self.counters.stranded.saturating_add((unsent - counted) as u32); + self.stranded = Some(Stranded { left: unsent, since: self.clock.nanos() }); } } if self.stranded.is_some() && self.link.is_up() { @@ -1268,7 +1210,7 @@ impl I219 { return Err(PassRefused::Stranded { left, after_nanos: waited }); } } - Ok(Pass { messages, causes, link_changed: self.link != before, rearmed }) + Ok(Pass { messages, causes, link_changed: self.link != before }) } /// How long until a pass has to begin whether or not a message came: when @@ -1288,39 +1230,6 @@ impl I219 { (self.tx_next + TX_RING - self.tx_clean) % TX_RING } - /// Reset the 82574 and bring it up again, as [`Self::open`] did, with - /// both rings empty and the multicast table as it stood. - fn rearm(&mut self) -> Result<(), Refusal> { - let unsent = self.unsent() as u32; - // The reset clears the statistics registers, so what they hold is - // taken first. - self.wire(); - let dwords = match self.part { - Part::E82574 => regs::MTA_DWORDS, - Part::I219 => regs::MTA_DWORDS_PCH, - }; - let mut table = [0u32; regs::MTA_DWORDS]; - for (entry, word) in table[..dwords].iter_mut().enumerate() { - *word = self.regs.read(regs::MTA + entry * 4); - } - let Programmed { mac: _, brought_up } = program(self.part, &self.regs, &self.clock, &self.dma)?; - for (entry, word) in table[..dwords].iter().enumerate() { - self.regs.write(regs::MTA + entry * 4, *word); - } - self.brought_up = brought_up; - self.rx_next = 0; - self.rx_tail = RX_RING - 1; - self.tx_next = 0; - self.tx_clean = 0; - self.counters.unsent = self.counters.unsent.saturating_add(unsent); - // The link change the bring-up itself made is acknowledged before the - // link is read: left in `ICR` it would be the next pass's reason to - // take back a ring that has frames in it again. - self.regs.write(regs::ICR, cause::LSC); - self.refresh_link(); - Ok(()) - } - /// Re-read `STATUS` and take what it says about the link. fn refresh_link(&mut self) { let status = self.regs.read(regs::STATUS); @@ -1408,7 +1317,7 @@ impl I219 { /// wrote back — `DD` is what [`Self::poll_rx`] took it on. The device /// touches neither until the tail passes it. fn give_back(&mut self, index: usize) { - publish_rx(&self.dma, index); + self.publish_rx(index); let mut tail = self.rx_tail; loop { let next = (tail + 1) % RX_RING; diff --git a/toyos-i219/src/stub.rs b/toyos-i219/src/stub.rs index 4882bcbfccd..4d14562c8a9 100644 --- a/toyos-i219/src/stub.rs +++ b/toyos-i219/src/stub.rs @@ -97,9 +97,9 @@ pub struct Permits { pub spurious_interrupts: bool, /// What a part does with the transmit descriptors it holds when its link /// goes away. Off: it sends them when the link returns, which is §10.2.7's - /// Defer Count on the 82574 — a transmit deferred because "The link is not - /// up". On: it never sends them, and takes no other, until it is reset, - /// which no document rules out of the PCH's MAC. + /// Defer Count — a transmit deferred because "The link is not up". On: it + /// never sends them, and takes no other, until it is reset: the part a + /// driver has to refuse. pub link_loss_strands_the_ring: bool, /// §10.2.4.1 case 3: "Interrupt was not asserted (ICR.INT_ASSERT=0): Read /// has no side affect." The document's own §7.4.5 says instead that "all @@ -1101,7 +1101,6 @@ impl Model { match reg { regs::CTRL => { let was = self.get(regs::CTRL); - let link_was = self.get(regs::STATUS) & status::LU; assert_eq!( value & CTRL_RESERVED_SET, CTRL_RESERVED_SET, @@ -1168,13 +1167,6 @@ impl Model { self.reset_reads = RESET_READS; } self.refresh_status(); - // §10.2.4.1: `LSC` "is set whenever the link status changes", - // and §4.6.3.2 has `LU` reflect the link "qualified with - // CTRL.SLU" — so a driver that writes `SLU` over a link the - // PHY already has, or resets it away, made a change itself. - if self.get(regs::STATUS) & status::LU != link_was { - self.raise(cause::LSC); - } } // §10.2.4.5: set, not assign — and "a PCIe interrupt is generated // whenever one of the bits in this register is set, and the diff --git a/toyos-i219/src/tests.rs b/toyos-i219/src/tests.rs index 2ce7e86f598..dd98a9a7ba1 100644 --- a/toyos-i219/src/tests.rs +++ b/toyos-i219/src/tests.rs @@ -611,12 +611,17 @@ fn a_frame_goes_out_whole() { const PARTS: [(Part, u32); 2] = [(Part::E82574, cause::TXDW | cause::TXQ0), (Part::I219, cause::TXDW)]; -/// Let a partner onto the wire and take the pass that sees the link. -fn link_up(nic: &Nic, driver: &mut Driver) { +/// Let a partner onto the wire, as far as a link. +fn partner_returns(nic: &Nic) { nic.set_link(true); if nic.part() == Part::I219 { nic.negotiation_settles(); } +} + +/// Let a partner onto the wire and take the pass that sees the link. +fn link_up(nic: &Nic, driver: &mut Driver) { + partner_returns(nic); one_pass(driver); assert!(driver.link().is_up(), "{}", nic.because("a partner on the wire raised no link")); } @@ -825,7 +830,7 @@ fn a_burst_past_the_ring_leaves_whole_on_the_room_it_is_told() { assert_eq!(on_the_wire, out, "{}", nic.because("what reached the wire is not the burst")); let counters = driver.counters(); assert!(counters.tx_wake_taken > 0, "{}", nic.because("no sleep was ended by the transmit cause")); - assert_eq!(counters.unsent, 0); + assert_eq!(counters.stranded, 0); } } @@ -848,121 +853,31 @@ fn a_link_that_is_down_has_no_room() { } } -/// A frame the 82574 holds for a link that went away is not one for the link -/// that comes: the pass that reads `LSC` over unsent descriptors resets the -/// function and brings it up again (§10.2.6.7: the head is software's to write -/// only after a reset). The ring is whole when the link returns, on a part -/// that would never have sent those frames and on one that would; the frames -/// are counted, and so are the ones the statistics registers held; the -/// multicast table and the receive ring are as they were. +/// §10.2.7's Defer Count: a transmit is deferred while "The link is not up". +/// The descriptors a link change found unsent stay the part's and are counted +/// once; there is no room while the link is down and nothing is owed; and a +/// part that sends them when the link returns goes on with a whole ring, +/// however long after. #[test] -fn a_ring_the_link_left_full_is_taken_back() { - const MDNS: [u8; 6] = [0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb]; - let mut reached_at_once = 0; - for (strands, seen_down) in [(true, true), (false, true), (true, false), (false, false)] { - let permits = Permits { link_loss_strands_the_ring: strands, ..Permits::default() }; - let (nic, mut driver) = held_with_a_link(26, Part::E82574, permits); - let why = |what: &str| nic.because(&format!("strands {strands}, seen down {seen_down}: {what}")); - // Three frames leave first, and nothing reads the statistics before - // the reset clears them. - for tag in 1..=3u8 { - let slot = driver.tx_reserve(100).expect("a ring with room"); - nic.put_bytes(slot.at, &frame(tag, 100)); - driver.tx_commit(slot); - } - settle(&nic, &mut driver); - assert_eq!(nic.sent().len(), 3); - driver.accept_multicast(MDNS); - let table: Vec = (0..regs::MTA_DWORDS).map(|dword| nic.peek(regs::MTA + dword * 4)).collect(); - - fill(&nic, &mut driver, 1); - nic.set_link(false); - if !seen_down { - // Down and up again between two passes: one `LSC`, and a link that - // reads up on both sides of it. - nic.set_link(true); - } - let pass = one_pass(&mut driver); - assert!(pass.rearmed, "{}", why("the ring was left with the link's unsent frames")); - assert_eq!(driver.counters().unsent, (TX_RING - 1) as u32, "{}", why("the frames given back were not counted")); - assert_eq!(driver.wire().sent, 3, "{}", why("the reset took the frames the MAC had counted")); - if seen_down { - assert_eq!(driver.tx_room(), 0, "{}", why("a link that is down had room")); - } - if driver.link().is_up() { - // The reset that left the PHY alone has its link back by the - // bring-up's own `SLU`, which is a link change of the driver's - // making: a frame in flight at the next pass is not given back - // for it. - let early = frame(0x70, 64); - let slot = driver.tx_reserve(early.len()).expect("a ring with room"); - nic.put_bytes(slot.at, &early); - driver.tx_commit(slot); - let next = one_pass(&mut driver); - assert!(!next.rearmed, "{}", why("the bring-up's own link change took the ring back again")); - nic.run(); - assert_eq!(nic.sent(), vec![early], "{}", why("the frame published after the reset did not leave")); - reached_at_once += 1; - } - - link_up(&nic, &mut driver); - assert_eq!(driver.tx_room(), TX_RING - 1, "{}", why("the ring did not come back whole")); - let after: Vec = (0..regs::MTA_DWORDS).map(|dword| nic.peek(regs::MTA + dword * 4)).collect(); - assert_eq!(after, table, "{}", why("the multicast table did not survive")); - - // A frame published now leaves, and it is the only one that does: the - // pass that follows a frame in flight resets nothing. - let payload = frame(0x77, 300); - let slot = driver.tx_reserve(payload.len()).expect("a ring with room"); - nic.put_bytes(slot.at, &payload); - driver.tx_commit(slot); - let next = one_pass(&mut driver); - assert!(!next.rearmed, "{}", why("the bring-up's own link change took the ring back again")); - let inbound = frame(0x78, 200); - nic.deliver(&inbound); - nic.run(); - assert_eq!(nic.sent(), vec![payload], "{}", why("what left is not the one frame published since")); - one_pass(&mut driver); - assert_eq!(drain(&nic, &mut driver), vec![inbound], "{}", why("the receive ring did not come back")); - assert_eq!(driver.counters().unsent, (TX_RING - 1) as u32); - } - assert!(reached_at_once > 0, "no part had its link back at the end of the reset"); -} - -/// The PCH's MAC is not reset over a ring it holds descriptors in: no Intel -/// document says that is safe on it. The descriptors a link change found -/// unsent stay its own and are counted once; there is no room while the link -/// is down and nothing is owed; and a part that sends them when the link -/// returns goes on with a whole ring, however long after. -#[test] -fn the_pch_mac_keeps_the_ring_a_link_change_left_full() { - for seen_down in [true, false] { +fn a_ring_a_link_change_left_full_stays_the_parts() { + for ((part, _), seen_down) in PARTS.into_iter().flat_map(|part| [(part, true), (part, false)]) { let permits = Permits { link_loss_strands_the_ring: false, ..Permits::default() }; - let (nic, mut driver) = held_with_a_link(28, Part::I219, permits); - let why = |what: &str| nic.because(&format!("seen down {seen_down}: {what}")); - let phy_resets = nic.phy_resets(); + let (nic, mut driver) = held_with_a_link(28, part, permits); + let why = |what: &str| nic.because(&format!("{part:?}, seen down {seen_down}: {what}")); assert_eq!(driver.pass_due_in(), None); let out = fill(&nic, &mut driver, 1); nic.set_link(false); if seen_down { - let pass = one_pass(&mut driver); - assert!(!pass.rearmed, "{}", why("the function was reset over a ring it held descriptors in")); + one_pass(&mut driver); assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32, "{}", why("the descriptors left were not counted")); assert_eq!(driver.tx_room(), 0, "{}", why("a link that is down had room")); assert_eq!(driver.pass_due_in(), None, "{}", why("a write-back was owed on a link that is down")); } - nic.set_link(true); - nic.negotiation_settles(); - let pass = one_pass(&mut driver); - assert!(!pass.rearmed && driver.link().is_up()); - assert_eq!(nic.phy_resets(), phy_resets, "{}", why("the function was reset over a ring it held descriptors in")); - assert_eq!( - (driver.counters().stranded, driver.counters().unsent), - ((TX_RING - 1) as u32, 0), - "{}", - why("the descriptors left were not counted once") - ); + partner_returns(&nic); + one_pass(&mut driver); + assert!(driver.link().is_up()); + assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32, "{}", why("the descriptors left were not counted once")); assert!(driver.pass_due_in().is_some_and(|due| due > 0), "{}", why("nothing was owed for the descriptors left")); // Written back with nobody asking for room: the pass that is due finds @@ -972,62 +887,49 @@ fn the_pch_mac_keeps_the_ring_a_link_change_left_full() { } assert_eq!(nic.sent(), out, "{}", why("what left when the link returned is not what the ring held")); nic.time_passes(STRANDED_DEADLINE_NANOS * 2); - let pass = driver.begin_pass().unwrap_or_else(|refused| panic!("{}", why(&format!("a ring written back whole: {refused}")))); - assert!(!pass.rearmed); + driver.begin_pass().unwrap_or_else(|refused| panic!("{}", why(&format!("a ring written back whole: {refused}")))); assert_eq!(driver.pass_due_in(), None, "{}", why("a write-back was owed on a ring written back whole")); assert_eq!(driver.tx_room(), TX_RING - 1); } } -/// A PCH MAC that has not written those descriptors back by the time the -/// slowest link the documents allow would have emptied the ring is handed up -/// by name — measured from the last link change read over them, and never -/// while the link is down. +/// A part that has not written those descriptors back by the time the slowest +/// link the documents allow would have emptied the ring is handed up by name — +/// measured from the last link change read over them, and never while the +/// link is down. #[test] -fn a_ring_the_pch_mac_never_writes_back_is_refused() { - let (nic, mut driver) = held_with_a_link(29, Part::I219, Permits::default()); - fill(&nic, &mut driver, 1); - nic.set_link(false); - one_pass(&mut driver); - nic.time_passes(STRANDED_DEADLINE_NANOS * 2); - one_pass(&mut driver); - link_up(&nic, &mut driver); - - // A second change over the same descriptors counts none of them again and - // is where the deadline runs from. - nic.time_passes(STRANDED_DEADLINE_NANOS / 2); - nic.set_link(false); - nic.set_link(true); - nic.negotiation_settles(); - one_pass(&mut driver); - assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32); - nic.time_passes(STRANDED_DEADLINE_NANOS * 3 / 4); - nic.run(); - one_pass(&mut driver); - assert_eq!(driver.tx_room(), 0, "{}", nic.because("a ring the part never wrote back had room")); - - let due = driver.pass_due_in().expect("a write-back is owed"); - assert!(due > 0 && due < STRANDED_DEADLINE_NANOS / 4); - nic.time_passes(due); - assert_eq!(driver.pass_due_in(), Some(0)); - let Err(PassRefused::Stranded { left, after_nanos }) = driver.begin_pass() else { - panic!("{}", nic.because("a ring the part never wrote back began a pass past its deadline")); - }; - assert_eq!(left, TX_RING - 1); - assert!(after_nanos >= STRANDED_DEADLINE_NANOS); -} +fn a_ring_never_written_back_is_refused() { + for (part, _) in PARTS { + let (nic, mut driver) = held_with_a_link(29, part, Permits::default()); + fill(&nic, &mut driver, 1); + nic.set_link(false); + one_pass(&mut driver); + nic.time_passes(STRANDED_DEADLINE_NANOS * 2); + one_pass(&mut driver); + link_up(&nic, &mut driver); -/// A function that does not come back from that reset is handed up by name, -/// and not driven on. -#[test] -fn a_ring_that_cannot_be_taken_back_is_refused() { - let (nic, mut driver) = held_with_a_link(27, Part::E82574, Permits::default()); - fill(&nic, &mut driver, 1); - nic.set_link(false); - nic.reset_never_clears(); - let Err(PassRefused::Rearm(Refusal::ResetUnfinished { .. })) = driver.begin_pass() else { - panic!("{}", nic.because("a reset that never cleared began a pass")); - }; + // A second change over the same descriptors counts none of them again and + // is where the deadline runs from. + nic.time_passes(STRANDED_DEADLINE_NANOS / 2); + nic.set_link(false); + partner_returns(&nic); + one_pass(&mut driver); + assert_eq!(driver.counters().stranded, (TX_RING - 1) as u32); + nic.time_passes(STRANDED_DEADLINE_NANOS * 3 / 4); + nic.run(); + one_pass(&mut driver); + assert_eq!(driver.tx_room(), 0, "{}", nic.because("a ring the part never wrote back had room")); + + let due = driver.pass_due_in().expect("a write-back is owed"); + assert!(due > 0 && due < STRANDED_DEADLINE_NANOS / 4); + nic.time_passes(due); + assert_eq!(driver.pass_due_in(), Some(0)); + let Err(PassRefused::Stranded { left, after_nanos }) = driver.begin_pass() else { + panic!("{}", nic.because("a ring the part never wrote back began a pass past its deadline")); + }; + assert_eq!(left, TX_RING - 1); + assert!(after_nanos >= STRANDED_DEADLINE_NANOS); + } } /// §7.2.4.2 lets the device hold transmit write-backs in a batch, so a later @@ -1300,7 +1202,6 @@ fn the_link_going_away_and_coming_back_is_seen() { let down = one_pass(&mut driver); assert!(down.causes & cause::LSC != 0, "{}", nic.because("no LSC for the link going away")); assert!(down.link_changed); - assert!(!down.rearmed, "{}", nic.because("a link change over an empty ring reset the function")); assert_eq!(driver.link(), Link::Down); nic.set_link(true); @@ -1348,29 +1249,24 @@ fn a_link_drop_does_not_lose_the_ring() { #[test] fn a_seeded_workload_loses_nothing_and_invents_nothing() { for seed in 1..=48u64 { - let nic = Nic::new(seed); + // A frame the part holds when its link goes is one it sends when the + // link returns (§10.2.7's Defer Count), so nothing is lost across one. + let permits = Permits { link_loss_strands_the_ring: false, ..Permits::default() }; + let nic = Nic::with(seed, Part::E82574, permits); let mut driver = open(&nic); nic.set_link(true); + one_pass(&mut driver); let mut expect_in: Vec> = Vec::new(); let mut expect_out: Vec> = Vec::new(); let mut got_in: Vec> = Vec::new(); let mut got_out: Vec> = Vec::new(); for round in 0..40u8 { + let inbound = frame(round + 1, 64 + round as usize * 13); + nic.deliver(&inbound); + expect_in.push(inbound); if round % 5 == 3 { - // A frame the part still holds when its link changes goes with - // the ring the driver takes back, so the wire is quiet first. - for _ in 0..8 { - nic.run(); - one_pass(&mut driver); - got_in.extend(drain(&nic, &mut driver)); - got_out.extend(nic.sent()); - } nic.set_link(round % 10 == 3); - } else { - let inbound = frame(round + 1, 64 + round as usize * 13); - nic.deliver(&inbound); - expect_in.push(inbound); } let outbound = frame(0x80 | round, 90 + round as usize * 11); if let Some(slot) = driver.tx_reserve(outbound.len()) { @@ -1379,15 +1275,11 @@ fn a_seeded_workload_loses_nothing_and_invents_nothing() { expect_out.push(outbound); } nic.run(); - let given_back = driver.counters().unsent; one_pass(&mut driver); - // Published into a link that had gone and the driver had not yet - // seen go: the newest frames, counted and never sent. - let given_back = (driver.counters().unsent - given_back) as usize; - expect_out.truncate(expect_out.len() - given_back); got_in.extend(drain(&nic, &mut driver)); got_out.extend(nic.sent()); } + nic.set_link(true); for _ in 0..64 { nic.run(); one_pass(&mut driver); diff --git a/userland/netstack/src/card.rs b/userland/netstack/src/card.rs index b74b2926503..b83a0aae0e1 100644 --- a/userland/netstack/src/card.rs +++ b/userland/netstack/src/card.rs @@ -63,10 +63,8 @@ impl Card { /// silently never arrives, so this dies where it can be read — once, for /// whichever driver is running. /// - /// So does an 82574 that did not come back from the reset a link change - /// over unsent frames costs it, and an I219 that kept the frames such a - /// change left in its ring past the driver's deadline: nothing drives - /// either from there. + /// So does an Intel part that kept the frames a link change left in its + /// transmit ring past the driver's deadline: nothing drives it from there. /// /// Answers the link where the pass found it changed; virtio reports none. pub fn begin_pass(&self) -> Option { @@ -122,7 +120,6 @@ impl Card { let (counters, wire) = nic.counts(); snap.put("descriptors.sent", counters.sent); snap.put("descriptors.received", counters.received); - snap.put("descriptors.unsent", counters.unsent); snap.put("descriptors.stranded", counters.stranded); snap.put("transmit.full", counters.tx_full); snap.put("transmit.wake_armed", counters.tx_wake_armed); diff --git a/userland/netstack/src/i219.rs b/userland/netstack/src/i219.rs index 3f35a4d0048..14f25d26f16 100644 --- a/userland/netstack/src/i219.rs +++ b/userland/netstack/src/i219.rs @@ -292,18 +292,8 @@ impl Nic { /// Take the interrupt, acknowledge its causes and refresh the link — and /// answer the link where this pass found it changed. - /// - /// A pass that reset the 82574 to take its transmit ring back says so, - /// with what the bring-up found the second time. pub fn begin_pass(&self) -> Result, toyos_i219::PassRefused> { let pass = self.driver.borrow_mut().begin_pass()?; - if pass.rearmed { - crate::say!( - "netstack: I219: the link changed over unsent frames, and the function was reset \ - to take its transmit ring back" - ); - say_brought_up(self.driver.borrow().brought_up()); - } Ok(pass.link_changed.then(|| self.link())) } @@ -396,15 +386,13 @@ impl Nic { crate::say!( "netstack: I219: refused {} over-length, {} errored, {} split and {} empty \ descriptor(s); {} overrun(s), {} descriptor-starvation report(s), \ - {} frame(s) given back unsent and {} left in the ring at a link change, \ - {} spurious interrupt(s)", + {} frame(s) left in the ring at a link change, {} spurious interrupt(s)", counters.over_length, counters.errored, counters.split, counters.empty, counters.overruns, counters.starved, - counters.unsent, counters.stranded, counters.spurious, ); From d7afcb3c3dc2cb607fa19881c7340a0273fb0cc3 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 01:27:04 +0200 Subject: [PATCH 6/7] The link-change issue says what the rule costs, what both parts' documents say, and who owes its instrument Review round 3 of #782, its NOTEs. The I219 datasheet has its own Defer Count (612523 9.5.4.6) and its own retry count (Power Management Control, PHY address 01, page 769, register 21, bits 8:1, default 0x0F), so the first unknown narrows to "no machine has read it". Recorded beside it: stale frames on the link that returns, or lost by the TNCRS reading; a deadline that does not bound a busy half-duplex segment; and a refusal that is safe only while netstack's row has no restart. The link partner a row commands is owed by stage 2 of the LAN track, which now says so. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...-read-an-intel-nic-across-a-link-change.md | 108 ++++++++++++------ issues/the-lan-is-not-yet-production-grade.md | 3 + 2 files changed, 74 insertions(+), 37 deletions(-) diff --git a/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md index 303705d471c..2ed51d7745f 100644 --- a/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md +++ b/issues/no-machine-has-read-an-intel-nic-across-a-link-change.md @@ -4,43 +4,42 @@ kind: tooling opened: 2026-10-09 --- -# No machine has read an Intel NIC across a link change, and two things about the I219 there are unknown +# No machine has read an Intel NIC across a link change A link that changes while the transmit ring holds unsent descriptors leaves them the part's, on the 82574 and on the I219 alike (`toyos-i219/src/lib.rs`, "A frame is offered only to a ring with room for it"): they are counted `descriptors.stranded`, the part is owed their -write-back within `STRANDED_DEADLINE_NANOS` (12.98 s, the slowest the 82574 -datasheet's §10.2.6.1 lets a full ring leave) of the link being read up, and a -part that has not written them back by then is refused by name, so netstack -ends with "cannot be driven on". Every reading of that is the driver's own -model. +write-back within `STRANDED_DEADLINE_NANOS` (12.98 s) of the link being read +up, and a part that has not written them back by then is refused by name, so +netstack ends with "cannot be driven on". Every reading of that is the +driver's own model. -**The 82574's document says it writes them back**: its Defer Count (§10.2.7) -counts a transmit deferred because "The link is not up". No 82574 is in -reach, and QEMU's `e1000e` is not in the harness. +**Both parts' documents say they keep the frame for the link that returns.** +The 82574 datasheet's Defer Count (317694 §10.2.7) and the I219 datasheet's +own (612523 §9.5.4.6) count a transmit deferred because "the link is not up". +Both give the same sixteen attempts the bound is built from: the 82574's +`TCTL.CT`, and the I219's Power Management Control register (PHY address 01, +page 769, register 21, bits 8:1, "Number of retries for a collided packet", +default 0x0F). No machine has read either part do it: no 82574 is in reach, +QEMU's `e1000e` is not in the harness, and no row can drop the T14's link. -**Of the I219 two things are unknown:** - -- **Whether it writes those descriptors back when the link returns.** The - Defer Count is the only sentence on the subject in any document read, and - it is the other part's. A part that does not is refused at the deadline. -- **Whether a reset over a live ring hangs it.** Nothing Intel publishes that - was found says it is safe or that it is not, which is why neither part is - reset to take a ring back. Read, and silent on it: the I219 datasheet - (612523 rev 2.02), the 500 Series on-package PCH datasheet volumes 1 and 2 - (631119-007 §18, 631120-002 §8), the 100 Series PCH datasheet volume 2 - (332691) and specification update (332692-015), the 400 Series PCH - datasheet volume 2 (620855-002) and specification update (620856-009), the - 300 Series PCH specification update (337349-012), the 7th generation U/Y - platform I/O specification update (334660-009), the I218 specification - update (rev 1.0), and the 82574 datasheet (317694 rev 3.4) and - specification update. intel.com's search finds no I219 specification - update, sighting or application note, and no specification update for the - 500 Series on-package PCH. The one piece of evidence the tree has is - `issues/the-t14-hung-after-rebooting-with-its-i219-faulted.md`: a T14 that - froze at reboot with that function left holding live rings, cause not - established. +**Whether a reset over a live ring hangs the I219 is unknown.** Nothing Intel +publishes that was found says it is safe or that it is not, which is why +neither part is reset to take a ring back. Read, and silent on it: the I219 +datasheet (612523 rev 2.02), the 500 Series on-package PCH datasheet volumes +1 and 2 (631119-007 §18, 631120-002 §8), the 100 Series PCH datasheet volume +2 (332691) and specification update (332692-015), the 400 Series PCH +datasheet volume 2 (620855-002) and specification update (620856-009), the +300 Series PCH specification update (337349-012), the 7th generation U/Y +platform I/O specification update (334660-009), the I218 specification +update (rev 1.0), and the 82574 datasheet (317694 rev 3.4) and specification +update. intel.com's search finds no I219 specification update, sighting or +application note, and no specification update for the 500 Series on-package +PCH. The one piece of evidence the tree has is +`issues/the-t14-hung-after-rebooting-with-its-i219-faulted.md`: a T14 that +froze at reboot with that function left holding live rings, cause not +established. **`open`'s own reset has the same exposure.** Every start of netstack resets the function before it programs it. On a netstack started again over a @@ -48,6 +47,33 @@ function that still holds published descriptors, `toyos_i219::quiesce` stops the receiver and the transmitter and the reset then goes out over that ring. Nothing guards it, because nothing found says what the guard is. +## What the rule costs + +Three weaknesses of leaving the ring to the part, each still true and each +read by the row the exit names: + +- **Stale frames leave on the link that returns**, up to + `toyos_i219::TX_RING - 1` of them and as old as the outage. Or they are + lost: the 82574 datasheet's §3.2 also has a transmission that "completes + successfully even if the PHY fails to indicate CRS", as in "a link down + situation", counted `TNCRS`, and a part that behaves so writes the + descriptors back with nothing sent. A TCP segment, a DHCP message with a + dead transaction id and an ARP request are each absorbed by the protocol + that receives them. The one with a cost is an ARP frame carrying the old + address onto a different network, which can displace a neighbour's entry + there until its owner speaks. +- **The deadline bounds collisions and not deference.** 12.98 s is a full + ring, sixteen attempts a frame, every attempt behind the largest back-off, + at 10 Mb/s. Nothing bounds how long a station defers to others on a busy + half-duplex segment, and no document read says how long after `STATUS.LU` + a part resumes. A part still deferring at the deadline is refused, and that + ends the machine's network. +- **The refusal is safe only while netstack's row has no `restart`.** + `[programs.netstack]` in `system.toml` carries none, so a refused part is + next reset by a `swap netstack` or a reboot. With a `restart` the refusal + becomes an automatic `open` over the ring the part kept, which is the reset + this issue does not know to be safe. + ## Owner Stage 2 of `issues/the-lan-is-not-yet-production-grade.md`. @@ -56,13 +82,21 @@ Stage 2 of `issues/the-lan-is-not-yet-production-grade.md`. None can be read yet without a hand on the cable: a link change under traffic on the T14 is its cable pulled or its switch port cycled, and no row -can do either. It becomes a row's when the T14's link partner is something a -row commands. That row fills the ring, drops the link and raises it, and -reads `descriptors.stranded` above 0 with netstack still serving (the part -writes them back) or netstack's "cannot be driven on" line naming the ring -(it does not, and the driver's answer is then a document or an owner's ruling -away). The reset over a live ring is read by a row that starts netstack again -under traffic and reboots: the machine comes back, or it does not. +can do either. The instrument is a link partner a row commands, owed by stage +2 of `issues/the-lan-is-not-yet-production-grade.md`, whose own exit ("link +survives unplug and replug") needs the same one. That row fills the ring, +drops the link and raises it, and reads: + +- `descriptors.stranded` above 0 with netstack still serving, and on the + partner whether the stranded frames arrived (stale frames) or `wire.sent` + stood still while the descriptors came back (lost by the `TNCRS` reading); + or netstack's "cannot be driven on" line naming the ring, where the part + wrote nothing back; +- the same at 10 Mb/s half duplex against a partner that keeps the segment + busy, for the deadline's false refusal; +- with netstack started again under traffic and the machine then rebooted, + whether it comes back: the reset over a live ring. Until that is read, + netstack's row takes no `restart`. An Intel document that states the I219's sequence for a reset over published descriptors closes the second unknown without a machine: `open` then does diff --git a/issues/the-lan-is-not-yet-production-grade.md b/issues/the-lan-is-not-yet-production-grade.md index 9d9217c48cb..744210c307f 100644 --- a/issues/the-lan-is-not-yet-production-grade.md +++ b/issues/the-lan-is-not-yet-production-grade.md @@ -35,6 +35,9 @@ why, and the boot still leases. - **1000 Mb/s.** Done: bench run 124 linked at 1000 full. - **Link events** arrive by interrupt and are recovered from without a restart: cable pulled and replugged, the partner rebooting, a flapping link. + Owed for it: a link partner a T14 row commands, so a row can drop the link + and raise it under traffic. Nothing has one, and + `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md` waits on it. - **Throughput** is measured against Ubuntu on the same machine and cable, with line rate (about 940 Mb/s of TCP) as the target: interrupt moderation (off today), ring sizes, checksum and segmentation offload. From ab49658de4d12d519f9e6de41b59068608a4f38c Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 01:28:02 +0200 Subject: [PATCH 7/7] toyos-i219: the two comments say what the I219's own datasheet says The header had the Defer Count as the 82574's alone and the bound's comment had the PCH's MAC publishing no transmit timing. 612523 9.5.4.6 is the I219's own Defer Count and its Power Management Control register carries its own retry count. Comments only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- toyos-i219/src/lib.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/toyos-i219/src/lib.rs b/toyos-i219/src/lib.rs index 1d20641aad2..9ed9f1df007 100644 --- a/toyos-i219/src/lib.rs +++ b/toyos-i219/src/lib.rs @@ -42,9 +42,10 @@ //! is. One the part already holds it keeps for the link that comes: §10.2.7's //! Defer Count has "A defer event occurs when the transmitter cannot //! immediately send a packet due to the medium being busy either because: -//! [...] The link is not up". That sentence is the 82574's, and the PCH's MAC -//! has none of its own. So the descriptors a link change finds unsent stay the -//! part's, on both: it is owed their write-back within +//! [...] The link is not up" of the 82574, and the *Intel Ethernet Connection +//! I219 Datasheet* (612523) §9.5.4.6 is the I219's own Defer Count, with the +//! same "or the link is not up". So the descriptors a link change finds unsent +//! stay the part's, on both: it is owed their write-back within //! `STRANDED_DEADLINE_NANOS` of the link being read up over them //! ([`I219::begin_pass`]), the ring goes on when they come, and a part that //! has not written them back by then is refused by name @@ -531,9 +532,10 @@ const MASTER_QUIESCE_DEADLINE_NANOS: u64 = 10_000_000; /// How long a part has to write back the descriptors a link change left in its /// transmit ring, from the pass that read the link up over them. /// -/// **One bound for both parts**, because both are driven through the one -/// `TCTL` this driver writes and §10.2.6.1 is the only document of it: the -/// PCH's MAC publishes no transmit timing of its own (631120 §8.2). +/// **One bound for both parts**, because their documents give the same +/// count: §10.2.6.1's `TCTL.CT` below, and on the I219 its Power Management +/// Control register (612523 §9.5.3.3, PHY address 01, page 769, register 21, +/// bits 8:1), "Number of retries for a collided packet", default 0x0F. /// /// **A ceiling on an event, and the slowest §10.2.6.1 lets a full ring leave**: /// [`TX_RING`]` - 1` frames of a whole buffer each, on a half-duplex link at