fix(ide): full-polling mode for IDE/ATAPI with scheduler-pump and IRQ hardening

Convert the IDE subsystem from interrupt-driven to full polling:
- IDEPacketCommand: poll BSY/DRQ/phase in setup() with transferIn/transferOut
  and bounded iteration (MAX_PHASE_ITERATIONS = 1<<20)
- IDEIdCommand: poll for completion and read data in setup()
- IDERWSectorsCommand.pollWait(): unchanged (already polling)
- All handleIRQ() methods become no-ops; CTR_NIEN disables device IRQs

IDEBus hardening:
- resetChannel(): softwareReset() + CTR_NIEN + clear currentCommand
  (prevents channel running with interrupts enabled after timeout recovery)
- executeAndWait(): sleep-poll loop instead of waitUntilFinished() to
  drive reschedule and dispatch pending interrupts
- process(): clear currentCommand on successful setup completion
- handleInterrupt(): deassert INTRQ for stray IRQs and no-op handlers
- probe(): keep CTR_NIEN after ATAPI identify fallback

vm-ints.asm: send EOI at interrupt level for legacy IDE lines (IRQ14/15)
to prevent PIC line latching from resume-overrun; other lines keep
deferred EOI required by level-triggered PCI devices.

VmIsolate: add scheduler-pump thread (user-mode, MIN_PRIORITY, yields
continuously) started from VmIsolate.run() covering both shell and
installer paths; self-heals if previous pump's isolate exited.

IDEConstants: remove unused CTR_IEN, add CTR_NIEN with Javadoc;
move IR_CD/IR_IO from IDEPacketCommand to IDEConstants; collapse
duplicate section headers and orphaned Javadoc.

Tests: add IDEPacketCommandTest (6 tests) with FakeIDEIO double;
fix FakeIDEIO.Phase.dataIn() intReason from 0x00 to IR_IO (was
causing test hangs by taking the write path).

Ref: #613 (diagnosis: kernel-mode IRQ resume-overrun drops
completions; polling avoids the dependency on yieldpoints in kernel
context but does not fix the assembly-level defects)

Author: LSantha

core/src/core/org/jnode/vm/isolate/VmIsolate.java

@@ -8,16 +8,16 @@
  * by the Free Software Foundation; either version 2.1 of the License, or
  * (at your option) any later version.
  *
- * This library is distributed in the hope that it will be useful, but 
+ * This library is distributed in the hope that it will be useful, but
  * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
- * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public 
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
  * License for more details.
  *
  * You should have received a copy of the GNU Lesser General Public License
- * along with this library; If not, write to the Free Software Foundation, Inc., 
+ * along with this library; If not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
- 
+
 package org.jnode.vm.isolate;
 
 import java.io.IOException;
@@ -460,7 +460,7 @@ public final void systemHalt(Isolate isolate, int status) {
     private void stopAllThreads() {
         // TODO - investigate it
         // TODO - this is probably unsafe because any of the threads being killed could
-        // be in the middle of updating a critical system data structure.  I'm also 
+        // be in the middle of updating a critical system data structure.  I'm also
         // unsure of the order in which we are killing the threads here.  It might be
         // better to kill the isolate's main thread first to give it the chance to
         // do a graceful shutdown.  (Stephen Crawley - 2008-11-08)
@@ -484,7 +484,7 @@ private void stopAllThreads() {
                 doExit();
             }
         } else {
-            // TODO - analyze this case      
+            // TODO - analyze this case
             doExit();
         }
     }
@@ -941,11 +941,67 @@ public void run() {
         }
     }
 
+144    /**
+     * The scheduler-pump thread (see {@link #startSchedulerPump()}), or
+     * {@code null} until first started. Volatile; deliberately NOT
+     * synchronized - the worst case of a racing check-then-act is two
+     * pump threads briefly coexisting, which is harmless (both are
+     * daemon threads at minimum priority doing nothing but yield()).
+     */
+    private static volatile Thread schedulerPump;
+
+    /**
+     * Start the scheduler-pump thread.
+     *
+     * This thread runs in USER mode and continuously yields: every
+     * Thread.yield() executes a yield point, which is the only mechanism
+     * that can run the scheduler's reschedule when every other thread is
+     * blocked without a timeout. Without it, a fully blocked system (all
+     * threads waiting on untimed monitors/queues) would never dispatch
+     * pending device interrupts nor expire timed waits again, because
+     * the kernel-mode idle thread cannot take yield points (a yield
+     * point taken in kernel mode terminates the system via
+     * yieldPointHandler_kernelCode).
+     *
+     * IMPORTANT: this must only ever be called from a user-mode (proclet)
+     * context. VmIsolate.run() qualifies, as it is invoked from
+     * IsolateThread, a normal java.lang.Thread. Code earlier in boot
+     * (VmSystem.initialize(), systemReadyForThreadSwitch()) runs in
+     * kernel context and MUST NOT start this thread.
+     *
+     * Historically this was started from CommandShell.run(), which left
+     * the installer path (org.jnode.install.Main bypasses CommandShell)
+     * running heavy IDE I/O without any scheduler pumping, deadlocking
+     * on the first IDE command.
+     */
+    private static void startSchedulerPump() {
+        final Thread t = schedulerPump;
+        if ((t != null) && t.isAlive()) {
+            return;
+        }
+        final Thread pump = new Thread(new Runnable() {
+            public void run() {
+                while (true) {
+                    Thread.yield();
+                }
+            }
+        }, "scheduler-pump");
+        pump.setDaemon(true);
+        pump.setPriority(Thread.MIN_PRIORITY);
+        pump.start();
+        schedulerPump = pump;
+    }
+
     /**
      * Run this isolate. This method is called from IsolateThread.
      */
     @PrivilegedActionPragma
     final void run(IsolateThread thread) {
+        // Ensure the scheduler-pump exists. Started here rather than in
+        // CommandShell so that every user-mode entry path (shell AND
+        // installer) is covered; self-heals if the isolate that hosted
+        // the previous pump has exited.
+        startSchedulerPump();
         try {
             // Set current
             IsolatedStaticData.current = VmIsolate.this;
@@ -954,7 +1010,7 @@ final void run(IsolateThread thread) {
             VmSystem.setOut(thread.getStdout());
             VmSystem.setErr(thread.getStderr());
             VmSystem.setIn(thread.getStdin());
-            
+
             // Set the isolate's properties to a copy of the initial properties passed
             // when the isolate was created.  (This needs to be done really early
             // via the IOContext switch to avoid NPEs when the native compiler,

core/src/native/x86/vm-ints.asm

@@ -379,20 +379,57 @@ timesliceHandler:
 ; -----------------------------------------------
 ; Handle an IRQ interrupt
 ; -----------------------------------------------
-def_irq_handler:
-	cmp GET_OLD_CS,USER_CS
-	jne def_irq_kernel
-	; Increment the appropriate IRQ counter and set threadSwitch indicator.
+; Count the IRQ, set the threadSwitch indicator and acknowledge it on the
+; PIC(s). Shared by the user-mode and kernel-mode interrupt paths.
+; ABP must point to the interrupt frame ([ABP+INTNO] = irq number).
+; The Java-level handler (and the old EOI in IRQThread.doHandle) run at
+; some arbitrary later reschedule; leaving the level in-service until then
+; can block lower-priority IRQ lines indefinitely, so EOI is sent here.
+def_irq_count_eoi:
 	mov AAX,[ABP+INTNO]
 	mov ADI,IRQCOUNT
 	inc dword [ADI+AAX*4+(VmArray_DATA_OFFSET*SLOT_SIZE)]
 	; Set thread switch indicator
 	or THREADSWITCHINDICATOR, VmProcessor_TSI_SWITCH_NEEDED
+	;
+	; Send the EOI right away ONLY for the edge-triggered legacy IDE
+	; lines (IRQ14 primary / IRQ15 secondary): their completion events
+	; were historically dropped by the resume-overrun logic, leaving
+	; commands waiting forever. Every other line keeps the original
+	; deferred EOI (done in IRQThread.doHandle), which is required for
+	; level-triggered PCI lines (e.g. PCnet) whose interrupt condition
+	; is only cleared by the device driver itself.
+	cmp dword [ABP+INTNO],14
+	je def_irq_eoi_ide
+	cmp dword [ABP+INTNO],15
+	je def_irq_eoi_ide
+	ret
+def_irq_eoi_ide:
+	mov ADX,[ABP+INTNO]
+	and ADX,1			; 14 -> 0 (master), 15 -> 1 (slave)
+	add ADX,0x60
+	mov al,dl
+	cmp dword [ABP+INTNO],14
+	jb def_irq_eoi_master
+	out 0xA0,al			; specific EOI on the slave PIC
+	mov al,0x62			; EOI the cascade line (IRQ2) on the master
+def_irq_eoi_master:
+	out 0x20,al
+	ret
+
+def_irq_handler:
+	cmp GET_OLD_CS,USER_CS
+	jne def_irq_kernel
+	call def_irq_count_eoi
 	; Done
 	ret
-	
+
 def_irq_kernel:
-	PRINT_STR irq_kernel_msg
+	; NOTE: the historical PRINT_STR irq_kernel_msg diagnostic was
+	; removed deliberately - it writes to the VGA text buffer from
+	; interrupt context and would flood during IRQ storms. The event
+	; is still counted (and now EOI'd for IDE lines) below.
+	call def_irq_count_eoi
 	ret
 	
 ; -----------------------------------------------

fs/build-tests.xml

@@ -38,6 +38,7 @@ help         Output these messages
 			<classpath refid="cp-test"/>
 			<formatter type="xml"/>
             <test name="org.jnode.test.driver.bus.ide.IDEDriveDescriptorTest" todir="${basedir}/build/reports/junit"/>
+            <test name="org.jnode.driver.bus.ide.command.IDEPacketCommandTest" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.filesystem.FSTestSuite" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.driver.tests.BlockDeviceAPITest" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.command.SyncCommandTest" todir="${basedir}/build/reports/junit"/>

fs/src/driver/org/jnode/driver/bus/ide/IDEBus.java

@@ -109,6 +109,10 @@ protected IDEBus(Device parent, boolean primary)
         irqRes = rm.claimIRQ(parent, io.getIrq(), this, true);
         // Reset the controller
         softwareReset();
+        // Run the IDE channels entirely interrupt-free: every command
+        // polls for completion (see the command classes). This removes
+        // the whole class of lost/misrouted completion interrupts.
+        io.setControlReg(CTR_NIEN);
         // Create and start the queue processor
         queueProcessor = new QueueProcessorThread<IDECommand>(name, commandQueue, this);
         queueProcessor.start();
@@ -128,6 +132,13 @@ public void execute(IDECommand command) {
      * Add the given command to the queue of commands to be executed and wait
      * for the command to finish.
      *
+     * The wait is implemented as a sleep-poll loop on purpose: every
+     * Thread.sleep suspends this thread, which runs a scheduler reschedule,
+     * which in turn dispatches pending device interrupts
+     * (IRQManager.dispatchInterrupts). Relying on Object.wait-notify alone
+     * can deadlock on a fully blocked system, where nothing else ever
+     * triggers a reschedule to deliver the interrupt that would notify us.
+     *
      * @param command
      * @param timeout Maximum time to wait
      * @throws InterruptedException
@@ -136,7 +147,13 @@ public void execute(IDECommand command) {
     public void executeAndWait(IDECommand command, long timeout)
         throws InterruptedException, TimeoutException {
         execute(command);
-        command.waitUntilFinished(timeout);
+        final long deadline = System.currentTimeMillis() + timeout;
+        while (!command.isFinished()) {
+            Thread.sleep(20);
+            if (!command.isFinished() && System.currentTimeMillis() >= deadline) {
+                throw new TimeoutException("timeout");
+            }
+        }
     }
 
     /**
@@ -156,9 +173,21 @@ public void stop() {
     public void handleInterrupt(int irq) {
         final IDECommand cmd = currentCommand;
         //log.debug("IDE IRQ " + irq + " cmd=" + cmd);
-        if (cmd != null) {
+        if (cmd == null) {
+            // Stray interrupt with no command in flight. Read the Status
+            // register to de-assert INTRQ: leaving the level asserted makes
+            // level-triggered lines (e.g. PCI IDE IRQ14/15) re-fire forever.
+            io.getStatusReg();
+            if (log.isDebugEnabled()) {
+                log.debug("Unknown IDE IRQ " + irq + " status 0x" + NumberUtils.hex(io.getAltStatusReg(), 2));
+            }
+        } else {
             try {
                 cmd.handleIRQ(this, io);
+                // Acknowledge any interrupt level the handler did not
+                // consume (e.g. no-op handlers for polling commands);
+                // an unquenchable level would otherwise refire forever.
+                io.getStatusReg();
                 if (cmd.isFinished()) {
                     this.currentCommand = null;
                 }
@@ -167,8 +196,6 @@ public void handleInterrupt(int irq) {
                 this.currentCommand = null;
                 cmd.setError(ERR_ABORT);
             }
-        } else if (log.isDebugEnabled()) {
-            log.debug("Unknown IDE IRQ " + irq + " status 0x" + NumberUtils.hex(io.getAltStatusReg(), 2));
         }
     }
 
@@ -189,8 +216,8 @@ public IDEDriveDescriptor probe(boolean master) throws InterruptedException {
             return null;
         }
 
-        // Interrupts enabled
-        io.setControlReg(CTR_BLANK);
+        // Interrupts stay disabled (full polling mode)
+        io.setControlReg(CTR_NIEN);
 
         // First try a normal IDE Identify command
         IDEIdCommand cmd = new IDEIdCommand(primary, master, false);
@@ -223,8 +250,8 @@ public IDEDriveDescriptor probe(boolean master) throws InterruptedException {
 
         // Clear any interrupts
         io.getStatusReg();
-        // Interrupts enabled
-        io.setControlReg(CTR_BLANK);
+        // Interrupts stay disabled (full polling mode)
+        io.setControlReg(CTR_NIEN);
 
         // IDE Identify failed, do an ATAPI Identify
         cmd = new IDEIdCommand(primary, master, true);
@@ -247,6 +274,27 @@ public IDEDriveDescriptor probe(boolean master) throws InterruptedException {
         }
     }
 
+    /**
+     * Reset both devices on this channel and clear any pending state.
+     * Used to recover from commands that timed out and may have left the
+     * device (emulator) with a latched interrupt or pending data phase.
+     *
+     * NOTE: there is an inherent, narrow race against the queue processor
+     * thread assigning {@link #currentCommand} in {@link #process()}.
+     * Callers must only invoke this method after the command they waited
+     * for has timed out AND must expect that a concurrently queued command
+     * may be aborted by the software reset; the retry issued by the caller
+     * re-serializes execution afterwards.
+     */
+    public void resetChannel() {
+        softwareReset();
+        // Re-disable interrupts after the reset; softwareReset() leaves
+        // the channel with CTR_BLANK (interrupts enabled) which is not
+        // what polling-mode code expects.
+        io.setControlReg(CTR_NIEN);
+        this.currentCommand = null;
+    }
+
     protected void softwareReset() {
         // Set reset
         io.setControlReg(CTR_SRST);
@@ -381,6 +429,7 @@ public void process(IDECommand cmd) /*throws Exception*/ {
         try {
             io.getStatusReg(); // Flush any pending IRQ
             cmd.setup(IDEBus.this, io);
+            this.currentCommand = null;
         } catch (TimeoutException ex) {
             log.error("Timeout in setup of " + cmd + ": " + ex.getMessage());
             if ((io.getAltStatusReg() & ST_ERROR) != 0) {

fs/src/driver/org/jnode/driver/bus/ide/IDEConstants.java

@@ -172,18 +172,33 @@ public interface IDEConstants {
     // Control bits
 
     /**
-     * Default control value
+     * Default control value (interrupts enabled, no reset)
      */
     public static final int CTR_BLANK = 0x00;
+
     /**
-     * Interrupt enable (0==enabled, 1==disabled)
+     * Device control register bit 1 (nIEN): when SET, interrupt requests
+     * from the drives on this channel are disabled. Despite its name, the
+     * historical constant CTR_IEN carried this same (inverted) value; it
+     * was unused and has been removed in favour of this correctly named
+     * constant.
      */
-    public static final int CTR_IEN = 0x02;
+    public static final int CTR_NIEN = 0x02;
+
     /**
      * Software reset (1==reset, 0==reset finished)
      */
     public static final int CTR_SRST = 0x04;
 
+    // --------------------------------
+    // Interrupt Reason bits (sector count register)
+
+    /** Interrupt Reason bit 0 (CoD): transfer is a command, not data. */
+    public static final int IR_CD = 0x01;
+
+    /** Interrupt Reason bit 1 (IO): transfer direction is device-to-host. */
+    public static final int IR_IO = 0x02;
+
     // --------------------------------
     // Timeout
 
@@ -204,16 +219,13 @@ public interface IDEConstants {
      */
     public static final int MAX_SECTOR_COUNT_48 = 65536;
 
-    // --------------------------------
-    // IDE sector maximum addresses
-
     /**
-     * Maximum sector for 28 bit addresses
+     * Maximum sector number for 28 bit addresses
      */
     public static final long MAX_SECTOR_28 = 0xfffffffL;
 
     /**
-     * Maximum sector for 48 bit addresses
+     * Maximum sector number for 48 bit addresses
      */
     public static final long MAX_SECTOR_48 = 0xfffffffffffffL;