Changeset 77010 in vbox

Timestamp:

Jan 27, 2019 9:45:05 AM (6 years ago)

Author:

vboxsync

Message:

DevATA: Clear interrupt when command register is written (ATA requirement). Also rely on the internal interrupt pending state more, to avoid unnecessary PDM calls (see bugref:5869).

Location:

trunk/src/VBox/Devices/Storage

Files:

• DevATA.cpp (modified) (4 diffs)
• VSCSI/VSCSILunMmc.cpp (modified) (2 diffs)

trunk/src/VBox/Devices/Storage/DevATA.cpp

@@ -1056 +1056 @@
 # endif /* IN_RING3 */
 
+/**
+ * Set the internal interrupt pending status, update INTREQ as appropriate.
+ *
+ * @param   s           Pointer to the ATA device state data.
+ */
 static void ataHCSetIRQ(ATADevState *s)
 {
@@ -1061 +1066 @@
     PPDMDEVINS pDevIns = ATADEVSTATE_2_DEVINS(s);
 
-    if (!(s->uATARegDevCtl & ATA_DEVCTL_DISABLE_IRQ))
-    {
-        Log2(("%s: LUN#%d asserting IRQ\n", __FUNCTION__, s->iLUN));
-        /* The BMDMA unit unconditionally sets BM_STATUS_INT if the interrupt
-         * line is asserted. It monitors the line for a rising edge. */
-        if (!s->fIrqPending)
+    if (!s->fIrqPending) {
+        if (!(s->uATARegDevCtl & ATA_DEVCTL_DISABLE_IRQ))
+        {
+            Log2(("%s: LUN#%d asserting IRQ\n", __FUNCTION__, s->iLUN));
+            /* The BMDMA unit unconditionally sets BM_STATUS_INT if the interrupt
+             * line is asserted. It monitors the line for a rising edge. */
             pCtl->BmDma.u8Status |= BM_STATUS_INT;
-        /* Only actually set the IRQ line if updating the currently selected drive. */
-        if (s == &pCtl->aIfs[pCtl->iSelectedIf])
-        {
-            /** @todo experiment with adaptive IRQ delivery: for reads it is
-             * better to wait for IRQ delivery, as it reduces latency. */
-            if (pCtl->irq == 16)
-                PDMDevHlpPCISetIrq(pDevIns, 0, 1);
-            else
-                PDMDevHlpISASetIrq(pDevIns, pCtl->irq, 1);
-        }
-    }
-    s->fIrqPending = true;
+            /* Only actually set the IRQ line if updating the currently selected drive. */
+            if (s == &pCtl->aIfs[pCtl->iSelectedIf])
+            {
+                /** @todo experiment with adaptive IRQ delivery: for reads it is
+                 * better to wait for IRQ delivery, as it reduces latency. */
+                if (pCtl->irq == 16)
+                    PDMDevHlpPCISetIrq(pDevIns, 0, 1);
+                else
+                    PDMDevHlpISASetIrq(pDevIns, pCtl->irq, 1);
+            }
+        }
+        s->fIrqPending = true;
+    }
 }
 
 #endif /* IN_RING0 || IN_RING3 */
 
+/**
+ * Clear the internal interrupt pending status, update INTREQ as appropriate.
+ *
+ * @param   s           Pointer to the ATA device state data.
+ */
 static void ataUnsetIRQ(ATADevState *s)
 {
@@ -1089 +1100 @@
     PPDMDEVINS pDevIns = ATADEVSTATE_2_DEVINS(s);
 
-    if (!(s->uATARegDevCtl & ATA_DEVCTL_DISABLE_IRQ))
-    {
-        Log2(("%s: LUN#%d deasserting IRQ\n", __FUNCTION__, s->iLUN));
-        /* Only actually unset the IRQ line if updating the currently selected drive. */
-        if (s == &pCtl->aIfs[pCtl->iSelectedIf])
-        {
-            if (pCtl->irq == 16)
-                PDMDevHlpPCISetIrq(pDevIns, 0, 0);
-            else
-                PDMDevHlpISASetIrq(pDevIns, pCtl->irq, 0);
-        }
-    }
-    s->fIrqPending = false;
+    if (s->fIrqPending) {
+        if (!(s->uATARegDevCtl & ATA_DEVCTL_DISABLE_IRQ))
+        {
+            Log2(("%s: LUN#%d deasserting IRQ\n", __FUNCTION__, s->iLUN));
+            /* Only actually unset the IRQ line if updating the currently selected drive. */
+            if (s == &pCtl->aIfs[pCtl->iSelectedIf])
+            {
+                if (pCtl->irq == 16)
+                    PDMDevHlpPCISetIrq(pDevIns, 0, 0);
+                else
+                    PDMDevHlpISASetIrq(pDevIns, pCtl->irq, 0);
+            }
+        }
+        s->fIrqPending = false;
+    }
 }
 
@@ -4412 +4425 @@
             return VINF_IOM_R3_IOPORT_WRITE;
 #else /* IN_RING3 */
+            ataUnsetIRQ(&pCtl->aIfs[pCtl->iSelectedIf]);
             ataR3ParseCmd(&pCtl->aIfs[pCtl->iSelectedIf], val);
 #endif /* !IN_RING3 */

trunk/src/VBox/Devices/Storage/VSCSI/VSCSILunMmc.cpp

@@ -60 +60 @@
 /** Unknown media type. */
 #define MMC_MEDIA_TYPE_UNKNOWN          0
+/** Medium is a DVD. */
+#define MMC_MEDIA_TYPE_DVD              2
 /** Door closed, no media. */
 #define MMC_MEDIA_TYPE_NO_DISC       0x70
@@ -586 +588 @@
         case 0x30:
         case 0x31:
+            /* For a CD, these must fail. */
+#if 0
+            if (pVScsiLunMmc->u32MediaTrackType != MMC_MEDIA_TYPE_DVD)
+                return vscsiLunReqSenseErrorSet(&pVScsiLunMmc->Core, pVScsiReq, SCSI_SENSE_ILLEGAL_REQUEST,
+                                                SCSI_ASC_CANNOT_READ_MEDIUM, SCSI_ASCQ_INCOMPATIBLE_FORMAT);
+#endif
+            RT_FALL_THRU();
         case 0xff:
             if (pVScsiReq->pbCDB[1] == 0)