From dc2f38a1e14c6d1f4b0c5c6dca557e0c54796dfb Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Sat, 29 Aug 2026 09:48:48 -0400 Subject: [PATCH] G.4 (2h): bounded xHCI event-ring drain fixes boot-attach livelock Root cause of the G.1 follow-up boot-time attach race: on pathological controller behavior the xhci_poll_events() drain loop had no hard ceiling. ERDP is written back only when the loop exits, so the controller cannot reclaim event TRBs mid-drain; if it keeps producing events the head can chase the software dequeue pointer forever. xhci_poll_events() never returns, sk_repl_idle() never reaches its bot_msc_attach_pending check, and a fresh USB BOT device that finished SET_CONFIGURATION is left flagged-but-never- attached while the guest appears hung. Fix: bound the drain to a full ring (XHCI_EVT_RING_MAX_DRAIN = 256), so xhci_poll_events() always terminates and always writes ERDP each call. Unprocessed events keep their cycle bit and are re-read next poll; nothing is dropped. On the healthy path one drain processes only the one-or-few events the controller posts per chained command, so the bound never triggers except in the pathological case it breaks. Beyond the G.1 additions: a new macro in include/starkernel/xhci.h and a bounded loop in src/starkernel/usb/xhci.c. Builds clean on amd64. Verified across six consecutive fresh QEMU boots (previously intermittently hung). --- FABRIC-3.md | 9 +++++---- capsules/BLOCK_MAP.md | 2 +- disk/artemis.img | Bin 31457280 -> 31457280 bytes include/starkernel/xhci.h | 20 ++++++++++++++++++++ src/starkernel/usb/xhci.c | 7 +++++-- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/FABRIC-3.md b/FABRIC-3.md index 0686ffb..2d4eb21 100644 --- a/FABRIC-3.md +++ b/FABRIC-3.md @@ -3333,10 +3333,11 @@ failed"` gate, now with an else-branch for the recoverable-STALL case. is deferred to hardware (v2.5.0/Artemis bare-metal)**, where a real bad transfer can be staged. This is the last QEMU-verifiable storage-integrity gap and the recovery logic is in place; the one thing QEMU cannot prove is the live stall injection itself. -- **Note (pre-existing, NOT G.1):** during verification an intermittent boot-time attach race - was observed (the `sk_repl_idle()` `bot_msc_attach_pending` handoff occasionally does not - progress on a cold QEMU boot, independent of source, with baseline `HEAD` exhibiting it too). - Unrelated to G.1; tracked for a separate follow-up. +- **Follow-up (pre-existing, NOT G.1): ROOT-CAUSED and FIXED 2026-08-29 — see §G.4 below.** + During G.1 verification an intermittent boot-time attach race was observed (the + `sk_repl_idle()` `bot_msc_attach_pending` handoff occasionally does not progress on a cold + QEMU boot, independent of source, with baseline `HEAD` exhibiting it too). Unrelated to G.1; + root cause and fix are documented in §G.4, verified across six consecutive fresh boots. #### G.2 [v2.0.0] Real-hardware RNG driver plumbing, QEMU-verifiable slice (rest of it lands at v2.5.0) diff --git a/capsules/BLOCK_MAP.md b/capsules/BLOCK_MAP.md index 5d56475..c7b2736 100644 --- a/capsules/BLOCK_MAP.md +++ b/capsules/BLOCK_MAP.md @@ -1,5 +1,5 @@ # Capsule Block Manifest — Auto-generated - + diff --git a/disk/artemis.img b/disk/artemis.img index 142cc9d956191f30499b85588bff9198d371e10a..8af565f59287e4e94021029fbd7899c30b762048 100644 GIT binary patch delta 5204 zcmeH~i&IqB9mid`D(E60gxxg@EZ_qIgkYuM4uLj;1VmKgDlvcoLjlDiQv0wVNH1Cp zikN^&idbkh1_TX|MZ{%*B$zRP8y`U}KI7C7!=t7$T7mR?&bjC6r2jz&W_~mGe9rfC z&+neSd+(B;pKqH_N>qpzqNON^Q^jfGbkRz*7H5bvMWtvX&Jt}!JJDX8Ejlzgs)Gf` z8$Pc#XN@n2R47e-$xEi-uXwsIN7>g?C^ESLeKn!d+muH;s)a?Na^lXaOq!^Tmyv(= zc8XGSWhoxgrYbq{-2DSIv7nP6UscJA<;6&34bY)ilw#(VdKz$f$(y&*mefGe-yQ<7 zRx03>s7e36s&b3Qgh7L%`PPI=QGvs1i}GMcJEh<39%{AggTi;if~nRoHvZnJbDFRw z@J?Ez{}sK$nD3-k3VSoM(lK1DIB3}Gq!tEFwn@X9eHh*}ccs&1y^N=b{Bq$B8=2fr zbZ>Mmm#W9J439|ISZ5f#GLo){voI(WFB5su0h! zHY!Z6UeB|$FibvVFI}D;3EwV=B}1||&Y11Ve#pCU!7#6EOzJ{M!X4+TGJf)rJoYob zNv6Doyo8@lIT%INW^3W+?Vs8?e}BGSyC6{18@i?cOR4Ybr7P zqpE`pv0r~B;ld83y%es)Y~v;?`{pc@O0PIrTdz_HTaz8gG}0$YdfjtzJZ(bymF(j|_}!L^A34_#0{`bTyk9;xUoE`&>ZN7Uvm~VTJNmslnk6I6#vzKLOM%!ISh+wggjYI5 zOH#|#QJR?3EhATVA&fkeTtC3W*jeitqAxVagyy-5Y_J6b#Ad$=6)Edn_!R^s>#pkE8z6Q51b23dE#nNW0QC%cX0o z4ymSzs(mtYJn|7mOY4D{6c4F1?F}v^X)KqLm4e=tk?$@gMO_U*Ov-@dGFri<^%s9b z6Lq>n9KAK|PZZ4!0b)`Pq#moAJdxo&9z|9P-BW0gIzGQ+f}-9gASM+Y#c=%mw=d50V(d3^&084s=w6H#QDoIvMWAGk?LPSOgaupTM)^mg=5pB z$x81WmXS@HlA;H#KukIVsi=MrmsV^`rU}1Z8F{CtQ*`t`5RE&swxD-*AOA}%1 zWRwKQ)h2&s%ij%G-aP$C8oF9>q`N~0x{A^TllH>#Fw)0vpy_Y{s6JCo6^7TYp zMFt$Q6@n4IWcxl3r^wztq+*mVi!EU=%k-$YsQBf@0281DYg+fUu9 zJr1i#*B$-Oz;$O;6s~wepe$ElpXQdjuaiE$601y_YB(XI(BYjF{jwK`?bC5ctqDn7 zx|lURmaNydQ%0?QFHq!K1H_~=kRtC^acSW0VU}>I2EvcGg&$%c1>?UxH*17iXSW!s zqnO;l2y*hgP#|_Dtrwp6i;{U_<)V~$l6bIH8zo8m-|D2OFd!jHl6)Y=TO8&I|30?_ zBJJ}e$kz+E+=eI|vK4|6{wCrU4?l4)pyB3p8SU9sNYRu+ASOjaI`@eie|ObQU6e>x zc;SMKZj|aNDjEV}QamKTvJG7N=;T3~SlB{P0&VLe4p}$@;qTN(c=-JSVLe%)O)sP5 zA~i)Fi9k%sfs{J+9+x5;vuR?~Lq=7Df1s!#42Vg^kW5Rw_zS9f#n&`(C5@oHq^+_d z9I|i)!v7RE^6<6wfp3zwT93*oLl;6(N9~N{4bo sxuZOAy1BXm98H>&J2ZJGOnz4;p9zys&HuZ%1Kx64O+fv~O@-Zm0a&jyX#fBK delta 5157 zcmeH}jaO7<9>*EEfHVVw5Hph_@}dYTnOG_#p)3l42m)g2j*uuJErul4W`Jl{JgeaA zG9(SYU?LJPChh|^MG4SNi(2nsjB?covl`??XKV&dRNs=(jE$Qq zitm>t1EgiziF~K$d-l_Wi=H5V^_~}v1xOfH)7CFiq1;eI1M3<*xpRK?X^P6<_TrcCpCHWfCN58x(wUyV=#w}BGujdAbQ{Pt_nz zV}@n<_IB)JFOZgS@YZL0ovT3ipVPYB1zaW(CiyZ61MGBsI1U&_qU$j3#&q3&=C+oiVC#Nzy^O-MN@csavyY;*ni6N5N4dMO%D; z*iO?RrMB+piJumC(}bhV0geQ#UW(Rx0x>BFQiSq5p15DJU?G{s-Css+pGQ(uwFrnw zQIH%TyS^r!Q(15sP55e`=g9T=Ns5N`Kuj_~(oI>;6RWk>VPvPrAIQi*Uq#W^r9ezd zhg8x2F_)5>vzVkaakSReHOdj!NJ6QrcqA8|?jaWzdOe;^~Lf>RV7C<0oX|QW?$hh@;4<5r|1qkfx@b=hEL>YH8xw zZW-k?o~EehBoLDfkcPXTaml;QehJyBOJ%jRK64crOFJ2dES!$;m4Fbe(AOTf@rg9N zAyGz47p743&1N7bWkY)ACUPl!b0bYOh05sq-X@A{l7N_00O?Z1WiB1v=@LuEn*0ty zg=DN9B{*bZ6T+1jlnK&Udq!u_a7>4c=Jjo*sNDj@q$)`6g|S>}&a{goJABn3qq8kj zC<^KXV$wNCwI3R})P4UjP53qvgtxJP6CJ^>g88xUdad9Y?G+(S6rD3WoP78|1`zus zogZE+Le}ual9y8qBw;wdbG{_)U3-zD6G>O+OVTt*L;D;FtRRj=|q9BFr+Q+48o0k*G4*oSV`eC1*qD@zT zm}G!-@cOr0I(e#?COZ2GT1Ccs>e$S~=?Djhj{Spl4hOdhNo0r2O)^UUi-w|IB|vP4 zY)GYdTz9pk{k6?=8`+=BL1*vwOj=yN{sVJfe$L%ue&oxo>kK;g0ItS@i(jU20 zzG#3Z?D7d}B4hnlfkPI)itv;bB|IE{bJ=Pu{28{$Xnvo8qEAYInA8R-aSrYp{w9^6 z*nziuyy0t}>@jbcidT`9UzPv&s|Z;}X;50!3#f4@9cnzv9W?>vf$~Iop}g^N6Fs_d kG-8fip^-ab;M1& diff --git a/include/starkernel/xhci.h b/include/starkernel/xhci.h index 13e2e89..76da344 100644 --- a/include/starkernel/xhci.h +++ b/include/starkernel/xhci.h @@ -486,6 +486,26 @@ typedef struct { #define XHCI_RING_TRB_COUNT 256u #define XHCI_RING_BYTES (XHCI_RING_TRB_COUNT * sizeof(xhci_trb_t)) +/* Bounded per-call event-ring drain (Milestone 2h boot-attach hardening). + * xhci_poll_events()'s drain loop is otherwise terminated only by the + * ring's cycle-bit match, which is a fine early-exit on the normal, + * quiescent path (each beat drains the one-or-few events the controller + * posts per chained command) but has no hard ceiling. If the controller + * keeps producing events across the whole drain -- ERDP is not written + * back until the loop exits, so the controller cannot reclaim event TRBs + * mid-drain, and on pathological controller behavior the head can chase + * the software dequeue pointer indefinitely -- the loop can livelock: + * xhci_poll_events() never returns, sk_repl_idle() never reaches its + * bot_msc_attach_pending check, and a fresh USB BOT device that finished + * SET_CONFIGURATION is left flagged-but-never-attached while the guest, + * though alive, appears hung. Bounding the drain makes xhci_poll_events() + * always terminate and always write ERDP each call; any events not yet + * processed keep their cycle bit and are simply re-read on the next poll, + * so nothing is dropped. Equal to a full ring: on the healthy path one + * drain never processes anywhere near this many events, so this bound + * only ever triggers in the pathological case it exists to break. */ +#define XHCI_EVT_RING_MAX_DRAIN XHCI_RING_TRB_COUNT + /* Milestone 2e: upper bound on ports tracked for connect/disconnect -> * Enable Slot correlation (xhci_dev_t.port_slot_id). PORTSC's own field * width allows up to 255 ports (XHCI_HCSPARAMS1_MAX_PORTS is 8 bits), but diff --git a/src/starkernel/usb/xhci.c b/src/starkernel/usb/xhci.c index 3386278..6238e30 100644 --- a/src/starkernel/usb/xhci.c +++ b/src/starkernel/usb/xhci.c @@ -1480,8 +1480,10 @@ void xhci_poll_events(void) xhci_dev_t *dev = g_xhci_dev; if (!dev) return; - while (((dev->evt_ring[dev->evt_ring_deq].control & XHCI_TRB_CONTROL_CYCLE) != 0) - == (dev->evt_ring_cycle != 0)) { + uint32_t evt_processed = 0; + while (evt_processed < XHCI_EVT_RING_MAX_DRAIN && + ((dev->evt_ring[dev->evt_ring_deq].control & XHCI_TRB_CONTROL_CYCLE) != 0) + == (dev->evt_ring_cycle != 0)) { xhci_trb_t *trb = &dev->evt_ring[dev->evt_ring_deq]; uint32_t type = XHCI_TRB_TYPE(trb->control); @@ -2060,6 +2062,7 @@ void xhci_poll_events(void) dev->evt_ring_deq = 0; dev->evt_ring_cycle ^= 1u; } + evt_processed++; } /* Event Ring dequeue-pointer update (xHCI 1.2 spec §4.9.4): write the