qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] lsi: misc fixes
@ 2016-08-18  7:31 Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 1/4] lsi: print register names in debug prints Hervé Poussineau
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Hervé Poussineau @ 2016-08-18  7:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Hervé Poussineau

Hi,

This patchset are some miscellaneous changes I did on LSI SCSI adapter:
- patch 1: some debugging help (register names)
- patch 2: removed one triggerable guest QEMU exit
- patch 3: compatibility with stock Windows 98 driver
- patch 4: compatibility with a real firmware

Note that patch 1 doesn't pass checkpatch.pl:
- lines with register names are > 80 chars. However, I prefer to keep 8 register names per line
- a case in a #define doesn't seem well supported. I preferred to mostly keep existing code

Hervé

Hervé Poussineau (4):
  lsi: print register names in debug prints
  lsi: do not exit QEMU if reading invalid register
  lsi: implement I/O memory space for Memory Move instructions
  lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register

 hw/scsi/lsi53c895a.c | 280 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 195 insertions(+), 85 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 1/4] lsi: print register names in debug prints
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
@ 2016-08-18  7:31 ` Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 2/4] lsi: do not exit QEMU if reading invalid register Hervé Poussineau
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hervé Poussineau @ 2016-08-18  7:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Hervé Poussineau

Modify lsi_reg_readb function to have a single exit point. Debug print can now
contain the returned value.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/scsi/lsi53c895a.c | 219 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 146 insertions(+), 73 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index df205cd..07dc73a 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -34,6 +34,23 @@ do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__); exit(1);} while
 do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__);} while (0)
 #endif
 
+#ifdef DEBUG_LSI_REG
+static const char *names[] = {
+    "SCNTL0", "SCNTL1", "SCNTL2", "SCNTL3", "SCID", "SXFER", "SDID", "GPREG",
+    "SFBR", "SOCL", "SSID", "SBCL", "DSTAT", "SSTAT0", "SSTAT1", "SSTAT2",
+    "DSA0", "DSA1", "DSA2", "DSA3", "ISTAT", "0x15", "0x16", "0x17",
+    "CTEST0", "CTEST1", "CTEST2", "CTEST3", "TEMP0", "TEMP1", "TEMP2", "TEMP3",
+    "DFIFO", "CTEST4", "CTEST5", "CTEST6", "DBC0", "DBC1", "DBC2", "DCMD",
+    "DNAD0", "DNAD1", "DNAD2", "DNAD3", "DSP0", "DSP1", "DSP2", "DSP3",
+    "DSPS0", "DSPS1", "DSPS2", "DSPS3", "SCRATCHA0", "SCRATCHA1", "SCRATCHA2", "SCRATCHA3",
+    "DMODE", "DIEN", "SBR", "DCNTL", "ADDER0", "ADDER1", "ADDER2", "ADDER3",
+    "SIEN0", "SIEN1", "SIST0", "SIST1", "SLPAR", "0x45", "MACNTL", "GPCNTL",
+    "STIME0", "STIME1", "RESPID", "0x4b", "STEST0", "STEST1", "STEST2", "STEST3",
+    "SIDL", "0x51", "0x52", "0x53", "SODL", "0x55", "0x56", "0x57",
+    "SBDL", "0x59", "0x5a", "0x5b", "SCRATCHB0", "SCRATCHB1", "SCRATCHB2", "SCRATCHB3",
+};
+#endif
+
 #define LSI_MAX_DEVS 7
 
 #define LSI_SCNTL0_TRG    0x01
@@ -1480,155 +1497,200 @@ again:
 
 static uint8_t lsi_reg_readb(LSIState *s, int offset)
 {
-    uint8_t tmp;
+    uint8_t ret;
+
 #define CASE_GET_REG24(name, addr) \
-    case addr: return s->name & 0xff; \
-    case addr + 1: return (s->name >> 8) & 0xff; \
-    case addr + 2: return (s->name >> 16) & 0xff;
+    case addr: ret = s->name & 0xff; break; \
+    case addr + 1: ret = (s->name >> 8) & 0xff; break; \
+    case addr + 2: ret = (s->name >> 16) & 0xff; break;
 
 #define CASE_GET_REG32(name, addr) \
-    case addr: return s->name & 0xff; \
-    case addr + 1: return (s->name >> 8) & 0xff; \
-    case addr + 2: return (s->name >> 16) & 0xff; \
-    case addr + 3: return (s->name >> 24) & 0xff;
+    case addr: ret = s->name & 0xff; break; \
+    case addr + 1: ret = (s->name >> 8) & 0xff; break; \
+    case addr + 2: ret = (s->name >> 16) & 0xff; break; \
+    case addr + 3: ret = (s->name >> 24) & 0xff; break;
 
-#ifdef DEBUG_LSI_REG
-    DPRINTF("Read reg %x\n", offset);
-#endif
     switch (offset) {
     case 0x00: /* SCNTL0 */
-        return s->scntl0;
+        ret = s->scntl0;
+        break;
     case 0x01: /* SCNTL1 */
-        return s->scntl1;
+        ret = s->scntl1;
+        break;
     case 0x02: /* SCNTL2 */
-        return s->scntl2;
+        ret = s->scntl2;
+        break;
     case 0x03: /* SCNTL3 */
-        return s->scntl3;
+        ret = s->scntl3;
+        break;
     case 0x04: /* SCID */
-        return s->scid;
+        ret = s->scid;
+        break;
     case 0x05: /* SXFER */
-        return s->sxfer;
+        ret = s->sxfer;
+        break;
     case 0x06: /* SDID */
-        return s->sdid;
+        ret = s->sdid;
+        break;
     case 0x07: /* GPREG0 */
-        return 0x7f;
+        ret = 0x7f;
+        break;
     case 0x08: /* Revision ID */
-        return 0x00;
+        ret = 0x00;
+        break;
     case 0x09: /* SOCL */
-        return s->socl;
+        ret = s->socl;
+        break;
     case 0xa: /* SSID */
-        return s->ssid;
+        ret = s->ssid;
+        break;
     case 0xb: /* SBCL */
         /* ??? This is not correct. However it's (hopefully) only
            used for diagnostics, so should be ok.  */
-        return 0;
+        ret = 0;
+        break;
     case 0xc: /* DSTAT */
-        tmp = s->dstat | LSI_DSTAT_DFE;
+        ret = s->dstat | LSI_DSTAT_DFE;
         if ((s->istat0 & LSI_ISTAT0_INTF) == 0)
             s->dstat = 0;
         lsi_update_irq(s);
-        return tmp;
+        break;
     case 0x0d: /* SSTAT0 */
-        return s->sstat0;
+        ret = s->sstat0;
+        break;
     case 0x0e: /* SSTAT1 */
-        return s->sstat1;
+        ret = s->sstat1;
+        break;
     case 0x0f: /* SSTAT2 */
-        return s->scntl1 & LSI_SCNTL1_CON ? 0 : 2;
+        ret = s->scntl1 & LSI_SCNTL1_CON ? 0 : 2;
+        break;
     CASE_GET_REG32(dsa, 0x10)
     case 0x14: /* ISTAT0 */
-        return s->istat0;
+        ret = s->istat0;
+        break;
     case 0x15: /* ISTAT1 */
-        return s->istat1;
+        ret = s->istat1;
+        break;
     case 0x16: /* MBOX0 */
-        return s->mbox0;
+        ret = s->mbox0;
+        break;
     case 0x17: /* MBOX1 */
-        return s->mbox1;
+        ret = s->mbox1;
+        break;
     case 0x18: /* CTEST0 */
-        return 0xff;
+        ret = 0xff;
+        break;
     case 0x19: /* CTEST1 */
-        return 0;
+        ret = 0;
+        break;
     case 0x1a: /* CTEST2 */
-        tmp = s->ctest2 | LSI_CTEST2_DACK | LSI_CTEST2_CM;
+        ret = s->ctest2 | LSI_CTEST2_DACK | LSI_CTEST2_CM;
         if (s->istat0 & LSI_ISTAT0_SIGP) {
             s->istat0 &= ~LSI_ISTAT0_SIGP;
-            tmp |= LSI_CTEST2_SIGP;
+            ret |= LSI_CTEST2_SIGP;
         }
-        return tmp;
+        break;
     case 0x1b: /* CTEST3 */
-        return s->ctest3;
+        ret = s->ctest3;
+        break;
     CASE_GET_REG32(temp, 0x1c)
     case 0x20: /* DFIFO */
-        return 0;
+        ret = 0;
+        break;
     case 0x21: /* CTEST4 */
-        return s->ctest4;
+        ret = s->ctest4;
+        break;
     case 0x22: /* CTEST5 */
-        return s->ctest5;
+        ret = s->ctest5;
+        break;
     case 0x23: /* CTEST6 */
-         return 0;
+        ret = 0;
+        break;
     CASE_GET_REG24(dbc, 0x24)
     case 0x27: /* DCMD */
-        return s->dcmd;
+        ret = s->dcmd;
+        break;
     CASE_GET_REG32(dnad, 0x28)
     CASE_GET_REG32(dsp, 0x2c)
     CASE_GET_REG32(dsps, 0x30)
     CASE_GET_REG32(scratch[0], 0x34)
     case 0x38: /* DMODE */
-        return s->dmode;
+        ret = s->dmode;
+        break;
     case 0x39: /* DIEN */
-        return s->dien;
+        ret = s->dien;
+        break;
     case 0x3a: /* SBR */
-        return s->sbr;
+        ret = s->sbr;
+        break;
     case 0x3b: /* DCNTL */
-        return s->dcntl;
+        ret = s->dcntl;
+        break;
     /* ADDER Output (Debug of relative jump address) */
     CASE_GET_REG32(adder, 0x3c)
     case 0x40: /* SIEN0 */
-        return s->sien0;
+        ret = s->sien0;
+        break;
     case 0x41: /* SIEN1 */
-        return s->sien1;
+        ret = s->sien1;
+        break;
     case 0x42: /* SIST0 */
-        tmp = s->sist0;
+        ret = s->sist0;
         s->sist0 = 0;
         lsi_update_irq(s);
-        return tmp;
+        break;
     case 0x43: /* SIST1 */
-        tmp = s->sist1;
+        ret = s->sist1;
         s->sist1 = 0;
         lsi_update_irq(s);
-        return tmp;
+        break;
     case 0x46: /* MACNTL */
-        return 0x0f;
+        ret = 0x0f;
+        break;
     case 0x47: /* GPCNTL0 */
-        return 0x0f;
+        ret = 0x0f;
+        break;
     case 0x48: /* STIME0 */
-        return s->stime0;
+        ret = s->stime0;
+        break;
     case 0x4a: /* RESPID0 */
-        return s->respid0;
+        ret = s->respid0;
+        break;
     case 0x4b: /* RESPID1 */
-        return s->respid1;
+        ret = s->respid1;
+        break;
     case 0x4d: /* STEST1 */
-        return s->stest1;
+        ret = s->stest1;
+        break;
     case 0x4e: /* STEST2 */
-        return s->stest2;
+        ret = s->stest2;
+        break;
     case 0x4f: /* STEST3 */
-        return s->stest3;
+        ret = s->stest3;
+        break;
     case 0x50: /* SIDL */
         /* This is needed by the linux drivers.  We currently only update it
            during the MSG IN phase.  */
-        return s->sidl;
+        ret = s->sidl;
+        break;
     case 0x52: /* STEST4 */
-        return 0xe0;
+        ret = 0xe0;
+        break;
     case 0x56: /* CCNTL0 */
-        return s->ccntl0;
+        ret = s->ccntl0;
+        break;
     case 0x57: /* CCNTL1 */
-        return s->ccntl1;
+        ret = s->ccntl1;
+        break;
     case 0x58: /* SBDL */
         /* Some drivers peek at the data bus during the MSG IN phase.  */
         if ((s->sstat1 & PHASE_MASK) == PHASE_MI)
             return s->msg[0];
-        return 0;
+        ret = 0;
+        break;
     case 0x59: /* SBDL high */
-        return 0;
+        ret = 0;
+        break;
     CASE_GET_REG32(mmrs, 0xa0)
     CASE_GET_REG32(mmws, 0xa4)
     CASE_GET_REG32(sfs, 0xa8)
@@ -1643,18 +1705,28 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
     CASE_GET_REG32(ia, 0xd4)
     CASE_GET_REG32(sbc, 0xd8)
     CASE_GET_REG32(csbc, 0xdc)
-    }
-    if (offset >= 0x5c && offset < 0xa0) {
+    case 0x5c ... 0x9f:
+    {
         int n;
         int shift;
         n = (offset - 0x58) >> 2;
         shift = (offset & 3) * 8;
-        return (s->scratch[n] >> shift) & 0xff;
+        ret = (s->scratch[n] >> shift) & 0xff;
+        break;
+    }
+    default:
+        BADF("readb 0x%x\n", offset);
+        exit(1);
     }
-    BADF("readb 0x%x\n", offset);
-    exit(1);
 #undef CASE_GET_REG24
 #undef CASE_GET_REG32
+
+#ifdef DEBUG_LSI_REG
+    DPRINTF("Read reg %s %x = %02x\n",
+            offset < ARRAY_SIZE(names) ? names[offset] : "???", offset, ret);
+#endif
+
+    return ret;
 }
 
 static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
@@ -1671,7 +1743,8 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
     case addr + 3: s->name &= 0x00ffffff; s->name |= val << 24; break;
 
 #ifdef DEBUG_LSI_REG
-    DPRINTF("Write reg %x = %02x\n", offset, val);
+    DPRINTF("Write reg %s %x = %02x\n",
+            offset < ARRAY_SIZE(names) ? names[offset] : "???", offset, val);
 #endif
     switch (offset) {
     case 0x00: /* SCNTL0 */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 2/4] lsi: do not exit QEMU if reading invalid register
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 1/4] lsi: print register names in debug prints Hervé Poussineau
@ 2016-08-18  7:31 ` Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 3/4] lsi: implement I/O memory space for Memory Move instructions Hervé Poussineau
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hervé Poussineau @ 2016-08-18  7:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Hervé Poussineau

When guest accesses invalid register, return 0xff instead of exiting.
Also add a log when reading or writing invalid registers.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/scsi/lsi53c895a.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 07dc73a..9d2e3eb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,6 +19,7 @@
 #include "hw/pci/pci.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/dma.h"
+#include "qemu/log.h"
 
 //#define DEBUG_LSI
 //#define DEBUG_LSI_REG
@@ -34,7 +35,6 @@ do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__); exit(1);} while
 do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__);} while (0)
 #endif
 
-#ifdef DEBUG_LSI_REG
 static const char *names[] = {
     "SCNTL0", "SCNTL1", "SCNTL2", "SCNTL3", "SCID", "SXFER", "SDID", "GPREG",
     "SFBR", "SOCL", "SSID", "SBCL", "DSTAT", "SSTAT0", "SSTAT1", "SSTAT2",
@@ -49,7 +49,6 @@ static const char *names[] = {
     "SIDL", "0x51", "0x52", "0x53", "SODL", "0x55", "0x56", "0x57",
     "SBDL", "0x59", "0x5a", "0x5b", "SCRATCHB0", "SCRATCHB1", "SCRATCHB2", "SCRATCHB3",
 };
-#endif
 
 #define LSI_MAX_DEVS 7
 
@@ -1715,8 +1714,14 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
         break;
     }
     default:
-        BADF("readb 0x%x\n", offset);
-        exit(1);
+    {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "lsi_scsi: invalid read from reg %s %x\n",
+                      offset < ARRAY_SIZE(names) ? names[offset] : "???",
+                      offset);
+        ret = 0xff;
+        break;
+    }
     }
 #undef CASE_GET_REG24
 #undef CASE_GET_REG32
@@ -1959,7 +1964,10 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             shift = (offset & 3) * 8;
             s->scratch[n] = deposit32(s->scratch[n], shift, 8, val);
         } else {
-            BADF("Unhandled writeb 0x%x = 0x%x\n", offset, val);
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "lsi_scsi: invalid write to reg %s %x (0x%02x)\n",
+                          offset < ARRAY_SIZE(names) ? names[offset] : "???",
+                          offset, val);
         }
     }
 #undef CASE_SET_REG24
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 3/4] lsi: implement I/O memory space for Memory Move instructions
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 1/4] lsi: print register names in debug prints Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 2/4] lsi: do not exit QEMU if reading invalid register Hervé Poussineau
@ 2016-08-18  7:31 ` Hervé Poussineau
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 4/4] lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register Hervé Poussineau
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hervé Poussineau @ 2016-08-18  7:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Hervé Poussineau

Memory Move instructions can read/write data either from PCI memory or from PCI I/O.
Implement second case.

Windows 98 now works with LSI 53C810A adapter.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/scsi/lsi53c895a.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 9d2e3eb..2e99d5e 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -210,6 +210,7 @@ typedef struct {
     MemoryRegion mmio_io;
     MemoryRegion ram_io;
     MemoryRegion io_io;
+    AddressSpace pci_io_as;
 
     int carry; /* ??? Should this be an a visible register somewhere?  */
     int status;
@@ -407,6 +408,30 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val);
 static void lsi_execute_script(LSIState *s);
 static void lsi_reselect(LSIState *s, lsi_request *p);
 
+static inline int lsi_mem_read(LSIState *s, dma_addr_t addr,
+                               void *buf, dma_addr_t len)
+{
+    if (s->dmode & LSI_DMODE_SIOM) {
+        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+                           buf, len);
+        return 0;
+    } else {
+        return pci_dma_read(PCI_DEVICE(s), addr, buf, len);
+    }
+}
+
+static inline int lsi_mem_write(LSIState *s, dma_addr_t addr,
+                                const void *buf, dma_addr_t len)
+{
+    if (s->dmode & LSI_DMODE_DIOM) {
+        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+                            buf, len);
+        return 0;
+    } else {
+        return pci_dma_write(PCI_DEVICE(s), addr, buf, len);
+    }
+}
+
 static inline uint32_t read_dword(LSIState *s, uint32_t addr)
 {
     uint32_t buf;
@@ -550,7 +575,6 @@ static void lsi_bad_selection(LSIState *s, uint32_t id)
 /* Initiate a SCSI layer data transfer.  */
 static void lsi_do_dma(LSIState *s, int out)
 {
-    PCIDevice *pci_dev;
     uint32_t count;
     dma_addr_t addr;
     SCSIDevice *dev;
@@ -562,7 +586,6 @@ static void lsi_do_dma(LSIState *s, int out)
         return;
     }
 
-    pci_dev = PCI_DEVICE(s);
     dev = s->current->req->dev;
     assert(dev);
 
@@ -588,9 +611,9 @@ static void lsi_do_dma(LSIState *s, int out)
     }
     /* ??? Set SFBR to first data byte.  */
     if (out) {
-        pci_dma_read(pci_dev, addr, s->current->dma_buf, count);
+        lsi_mem_read(s, addr, s->current->dma_buf, count);
     } else {
-        pci_dma_write(pci_dev, addr, s->current->dma_buf, count);
+        lsi_mem_write(s, addr, s->current->dma_buf, count);
     }
     s->current->dma_len -= count;
     if (s->current->dma_len == 0) {
@@ -1022,15 +1045,14 @@ bad:
 #define LSI_BUF_SIZE 4096
 static void lsi_memcpy(LSIState *s, uint32_t dest, uint32_t src, int count)
 {
-    PCIDevice *d = PCI_DEVICE(s);
     int n;
     uint8_t buf[LSI_BUF_SIZE];
 
     DPRINTF("memcpy dest 0x%08x src 0x%08x count %d\n", dest, src, count);
     while (count) {
         n = (count > LSI_BUF_SIZE) ? LSI_BUF_SIZE : count;
-        pci_dma_read(d, src, buf, n);
-        pci_dma_write(d, dest, buf, n);
+        lsi_mem_read(s, src, buf, n);
+        lsi_mem_write(s, dest, buf, n);
         src += n;
         dest += n;
         count -= n;
@@ -1877,9 +1899,6 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
     CASE_SET_REG32(dsps, 0x30)
     CASE_SET_REG32(scratch[0], 0x34)
     case 0x38: /* DMODE */
-        if (val & (LSI_DMODE_SIOM | LSI_DMODE_DIOM)) {
-            BADF("IO mappings not implemented\n");
-        }
         s->dmode = val;
         break;
     case 0x39: /* DIEN */
@@ -2189,6 +2208,8 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
                           "lsi-io", 256);
 
+    address_space_init(&s->pci_io_as, pci_address_space_io(dev), "lsi-pci-io");
+
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_io);
     pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mmio_io);
     pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->ram_io);
@@ -2200,6 +2221,13 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
     }
 }
 
+static void lsi_scsi_unrealize(DeviceState *dev, Error **errp)
+{
+    LSIState *s = LSI53C895A(dev);
+
+    address_space_destroy(&s->pci_io_as);
+}
+
 static void lsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2210,6 +2238,7 @@ static void lsi_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_LSI_53C895A;
     k->class_id = PCI_CLASS_STORAGE_SCSI;
     k->subsystem_id = 0x1000;
+    dc->unrealize = lsi_scsi_unrealize;
     dc->reset = lsi_scsi_reset;
     dc->vmsd = &vmstate_lsi_scsi;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 4/4] lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
                   ` (2 preceding siblings ...)
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 3/4] lsi: implement I/O memory space for Memory Move instructions Hervé Poussineau
@ 2016-08-18  7:31 ` Hervé Poussineau
  2016-08-18  7:41 ` [Qemu-devel] [PATCH 0/4] lsi: misc fixes no-reply
  2016-08-18  8:47 ` Paolo Bonzini
  5 siblings, 0 replies; 7+ messages in thread
From: Hervé Poussineau @ 2016-08-18  7:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Hervé Poussineau

53C895A datasheet says:
"This bit (DFE) is a pure status bit and will not cause an interrupt"

This bit is already auto-generated in lsi_read_reg when reading the DSTAT register.

This fixes IBM RS/6000 7020 firmware, which is:
- resetting the adapter
- enabling all interrupt sources (including DIP, ie interrupts from DSTAT)
- waiting for ISTAT0 to become 0 (including DIP=0, ie no interrupt coming from DSTAT)

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/scsi/lsi53c895a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 2e99d5e..feb1191 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -326,7 +326,7 @@ static void lsi_soft_reset(LSIState *s)
     s->istat0 = 0;
     s->istat1 = 0;
     s->dcmd = 0x40;
-    s->dstat = LSI_DSTAT_DFE;
+    s->dstat = 0;
     s->dien = 0;
     s->sist0 = 0;
     s->sist1 = 0;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] lsi: misc fixes
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
                   ` (3 preceding siblings ...)
  2016-08-18  7:31 ` [Qemu-devel] [PATCH 4/4] lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register Hervé Poussineau
@ 2016-08-18  7:41 ` no-reply
  2016-08-18  8:47 ` Paolo Bonzini
  5 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2016-08-18  7:41 UTC (permalink / raw)
  To: hpoussin; +Cc: famz, pbonzini, qemu-devel

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Message-id: 1471505489-1221-1-git-send-email-hpoussin@reactos.org
Subject: [Qemu-devel] [PATCH 0/4] lsi: misc fixes
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1471505489-1221-1-git-send-email-hpoussin@reactos.org -> patchew/1471505489-1221-1-git-send-email-hpoussin@reactos.org
Switched to a new branch 'test'
0734350 lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register
f8e5db3 lsi: implement I/O memory space for Memory Move instructions
08becc1 lsi: do not exit QEMU if reading invalid register
17693be lsi: print register names in debug prints

=== OUTPUT BEGIN ===
Checking PATCH 1/4: lsi: print register names in debug prints...
ERROR: line over 90 characters
#31: FILE: hw/scsi/lsi53c895a.c:45:
+    "DSPS0", "DSPS1", "DSPS2", "DSPS3", "SCRATCHA0", "SCRATCHA1", "SCRATCHA2", "SCRATCHA3",

WARNING: line over 80 characters
#34: FILE: hw/scsi/lsi53c895a.c:48:
+    "STIME0", "STIME1", "RESPID", "0x4b", "STEST0", "STEST1", "STEST2", "STEST3",

WARNING: line over 80 characters
#36: FILE: hw/scsi/lsi53c895a.c:50:
+    "SBDL", "0x59", "0x5a", "0x5b", "SCRATCHB0", "SCRATCHB1", "SCRATCHB2", "SCRATCHB3",

ERROR: trailing statements should be on next line
#54: FILE: hw/scsi/lsi53c895a.c:1503:
+    case addr: ret = s->name & 0xff; break; \

ERROR: trailing statements should be on next line
#55: FILE: hw/scsi/lsi53c895a.c:1504:
+    case addr + 1: ret = (s->name >> 8) & 0xff; break; \

ERROR: trailing statements should be on next line
#56: FILE: hw/scsi/lsi53c895a.c:1505:
+    case addr + 2: ret = (s->name >> 16) & 0xff; break;

ERROR: trailing statements should be on next line
#63: FILE: hw/scsi/lsi53c895a.c:1508:
+    case addr: ret = s->name & 0xff; break; \

ERROR: trailing statements should be on next line
#64: FILE: hw/scsi/lsi53c895a.c:1509:
+    case addr + 1: ret = (s->name >> 8) & 0xff; break; \

ERROR: trailing statements should be on next line
#65: FILE: hw/scsi/lsi53c895a.c:1510:
+    case addr + 2: ret = (s->name >> 16) & 0xff; break; \

ERROR: trailing statements should be on next line
#66: FILE: hw/scsi/lsi53c895a.c:1511:
+    case addr + 3: ret = (s->name >> 24) & 0xff; break;

total: 8 errors, 2 warnings, 332 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/4: lsi: do not exit QEMU if reading invalid register...
Checking PATCH 3/4: lsi: implement I/O memory space for Memory Move instructions...
Checking PATCH 4/4: lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] lsi: misc fixes
  2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
                   ` (4 preceding siblings ...)
  2016-08-18  7:41 ` [Qemu-devel] [PATCH 0/4] lsi: misc fixes no-reply
@ 2016-08-18  8:47 ` Paolo Bonzini
  5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2016-08-18  8:47 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: qemu-devel



On 18/08/2016 09:31, Hervé Poussineau wrote:
> Hi,
> 
> This patchset are some miscellaneous changes I did on LSI SCSI adapter:
> - patch 1: some debugging help (register names)
> - patch 2: removed one triggerable guest QEMU exit
> - patch 3: compatibility with stock Windows 98 driver
> - patch 4: compatibility with a real firmware
> 
> Note that patch 1 doesn't pass checkpatch.pl:
> - lines with register names are > 80 chars. However, I prefer to keep 8 register names per line
> - a case in a #define doesn't seem well supported. I preferred to mostly keep existing code

Queued for 2.8, thanks.

Paolo

> Hervé
> 
> Hervé Poussineau (4):
>   lsi: print register names in debug prints
>   lsi: do not exit QEMU if reading invalid register
>   lsi: implement I/O memory space for Memory Move instructions
>   lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register
> 
>  hw/scsi/lsi53c895a.c | 280 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 195 insertions(+), 85 deletions(-)
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-18  8:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-18  7:31 [Qemu-devel] [PATCH 0/4] lsi: misc fixes Hervé Poussineau
2016-08-18  7:31 ` [Qemu-devel] [PATCH 1/4] lsi: print register names in debug prints Hervé Poussineau
2016-08-18  7:31 ` [Qemu-devel] [PATCH 2/4] lsi: do not exit QEMU if reading invalid register Hervé Poussineau
2016-08-18  7:31 ` [Qemu-devel] [PATCH 3/4] lsi: implement I/O memory space for Memory Move instructions Hervé Poussineau
2016-08-18  7:31 ` [Qemu-devel] [PATCH 4/4] lsi: never set DMA FIFO Empty (DFE) bit in DSTAT register Hervé Poussineau
2016-08-18  7:41 ` [Qemu-devel] [PATCH 0/4] lsi: misc fixes no-reply
2016-08-18  8:47 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).