qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 12/13] hw/ide/sii3112: Reuse PCIIDEState::bmdma_ops

From: Mark Cave-Ayland
Subject: Re: [PATCH 12/13] hw/ide/sii3112: Reuse PCIIDEState::bmdma_ops
Date: Wed, 26 Apr 2023 12:44:29 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 
On 22/04/2023 16:07, Bernhard Beschow wrote:


Allows to unexport bmdma_addr_ioport_ops and models TYPE_SII3112_PCI as a
standard-compliant PCI IDE device.
Nice! I think it's worth adding a brief mention that you've added BMDMA trace-events, but otherwise looks sensible to me. 


Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
  include/hw/ide/pci.h |  1 -
  hw/ide/pci.c         |  2 +-
  hw/ide/sii3112.c     | 94 ++++++++++++++++++++++++++------------------
  hw/ide/trace-events  |  6 ++-
  4 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index dbb4b13161..81e0370202 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -59,7 +59,6 @@ struct PCIIDEState {
  void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
  void bmdma_init_ops(PCIIDEState *d, const MemoryRegionOps *bmdma_ops);
  void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
-extern MemoryRegionOps bmdma_addr_ioport_ops;
  void pci_ide_create_devs(PCIDevice *dev);
#endif 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 97ccc75aa6..3539b162b7 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -342,7 +342,7 @@ static void bmdma_addr_write(void *opaque, hwaddr addr,
      bm->addr |= ((data & mask) << shift) & ~3;
  }
-MemoryRegionOps bmdma_addr_ioport_ops = { 
+static MemoryRegionOps bmdma_addr_ioport_ops = {
      .read = bmdma_addr_read,
      .write = bmdma_addr_write,
      .endianness = DEVICE_LITTLE_ENDIAN,
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 9cf920369f..373c0dd1ee 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -34,47 +34,73 @@ struct SiI3112PCIState {
      SiI3112Regs regs[2];
  };
-/* The sii3112_reg_read and sii3112_reg_write functions implement the 
- * Internal Register Space - BAR5 (section 6.7 of the data sheet).
- */
-
-static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
-                                unsigned int size)
+static uint64_t sii3112_bmdma_read(void *opaque, hwaddr addr, unsigned int 
size)
  {
-    SiI3112PCIState *d = opaque;
+    BMDMAState *bm = opaque;
+    SiI3112PCIState *d = SII3112_PCI(bm->pci_dev);
+    int i = (bm == &bm->pci_dev->bmdma[0]) ? 0 : 1;
      uint64_t val;
switch (addr) { 
      case 0x00:
-        val = d->i.bmdma[0].cmd;
+        val = bm->cmd;
          break;
      case 0x01:
-        val = d->regs[0].swdata;
+        val = d->regs[i].swdata;
          break;
      case 0x02:
-        val = d->i.bmdma[0].status;
+        val = bm->status;
          break;
      case 0x03:
          val = 0;
          break;
-    case 0x04 ... 0x07:
-        val = bmdma_addr_ioport_ops.read(&d->i.bmdma[0], addr - 4, size);
-        break;
-    case 0x08:
-        val = d->i.bmdma[1].cmd;
+    default:
+        val = 0;
          break;
-    case 0x09:
-        val = d->regs[1].swdata;
+    }
+    trace_sii3112_bmdma_read(size, addr, val);
+    return val;
+}
+
+static void sii3112_bmdma_write(void *opaque, hwaddr addr,
+                                uint64_t val, unsigned int size)
+{
+    BMDMAState *bm = opaque;
+    SiI3112PCIState *d = SII3112_PCI(bm->pci_dev);
+    int i = (bm == &bm->pci_dev->bmdma[0]) ? 0 : 1;
+
+    trace_sii3112_bmdma_write(size, addr, val);
+    switch (addr) {
+    case 0x00:
+        bmdma_cmd_writeb(bm, val);
          break;
-    case 0x0a:
-        val = d->i.bmdma[1].status;
+    case 0x01:
+        d->regs[i].swdata = val & 0x3f;
          break;
-    case 0x0b:
-        val = 0;
+    case 0x02:
+        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 6);
          break;
-    case 0x0c ... 0x0f:
-        val = bmdma_addr_ioport_ops.read(&d->i.bmdma[1], addr - 12, size);
+    default:
          break;
+    }
+}
+
+static const MemoryRegionOps sii3112_bmdma_ops = {
+    .read = sii3112_bmdma_read,
+    .write = sii3112_bmdma_write,
+};
+
+/* The sii3112_reg_read and sii3112_reg_write functions implement the
+ * Internal Register Space - BAR5 (section 6.7 of the data sheet).
+ */
+
+static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
+                                unsigned int size)
+{
+    SiI3112PCIState *d = opaque;
+    uint64_t val;
+
+    switch (addr) {
      case 0x10:
          val = d->i.bmdma[0].cmd;
          val |= (d->regs[0].confstat & (1UL << 11) ? (1 << 4) : 0); 
/*SATAINT0*/
@@ -127,38 +153,26 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
trace_sii3112_write(size, addr, val); 
      switch (addr) {
-    case 0x00:
      case 0x10:
          bmdma_cmd_writeb(&d->i.bmdma[0], val);
          break;
-    case 0x01:
      case 0x11:
          d->regs[0].swdata = val & 0x3f;
          break;
-    case 0x02:
      case 0x12:
          d->i.bmdma[0].status = (val & 0x60) | (d->i.bmdma[0].status & 1) |
                                 (d->i.bmdma[0].status & ~val & 6);
          break;
-    case 0x04 ... 0x07:
-        bmdma_addr_ioport_ops.write(&d->i.bmdma[0], addr - 4, val, size);
-        break;
-    case 0x08:
      case 0x18:
          bmdma_cmd_writeb(&d->i.bmdma[1], val);
          break;
-    case 0x09:
      case 0x19:
          d->regs[1].swdata = val & 0x3f;
          break;
-    case 0x0a:
      case 0x1a:
          d->i.bmdma[1].status = (val & 0x60) | (d->i.bmdma[1].status & 1) |
                                 (d->i.bmdma[1].status & ~val & 6);
          break;
-    case 0x0c ... 0x0f:
-        bmdma_addr_ioport_ops.write(&d->i.bmdma[1], addr - 12, val, size);
-        break;
      case 0x100:
          d->regs[0].scontrol = val & 0xfff;
          if (val & 1) {
@@ -240,6 +254,9 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
**errp)
      pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_IO, &s->data_ops[1]);
      pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &s->cmd_ops[1]);
+ bmdma_init_ops(s, &sii3112_bmdma_ops); 
+    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &s->bmdma_ops);
+
      /* BAR5 is in PCI memory space */
      memory_region_init_io(&d->mmio, OBJECT(d), &sii3112_reg_ops, d,
                           "sii3112.bar5", 0x200);
@@ -262,10 +279,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
**errp)
      memory_region_init_alias(mr, OBJECT(d), "sii3112.bar3", &s->cmd_ops[1], 0,
                               memory_region_size(&s->cmd_ops[1]));
      memory_region_add_subregion_overlap(&d->mmio, 0xc8, mr, 1);
-
      mr = g_new(MemoryRegion, 1);
-    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0, 16);
-    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &s->bmdma_ops, 0,
+                             memory_region_size(&s->bmdma_ops));
+    memory_region_add_subregion_overlap(&d->mmio, 0x0, mr, 1);
qdev_init_gpio_in(ds, sii3112_set_irq, 2); 
      for (i = 0; i < 2; i++) {
@@ -287,7 +304,6 @@ static void sii3112_pci_class_init(ObjectClass *klass, void 
*data)
      pd->class_id = PCI_CLASS_STORAGE_RAID;
      pd->revision = 1;
      pd->realize = sii3112_pci_realize;
-    pd->exit = NULL;
      dc->reset = sii3112_reset;
      dc->vmsd = NULL;
      dc->desc = "SiI3112A SATA controller";
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 57042cafdd..a479525e38 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -38,8 +38,10 @@ bmdma_read(uint64_t addr, uint8_t val) "bmdma: readb 0x%"PRIx64" 
: 0x%02x"
  bmdma_write(uint64_t addr, uint64_t val) "bmdma: writeb 0x%"PRIx64" : 
0x%02"PRIx64
# sii3112.c 
-sii3112_read(int size, uint64_t addr, uint64_t val) "bmdma: read (size %d) 
0x%"PRIx64" : 0x%02"PRIx64
-sii3112_write(int size, uint64_t addr, uint64_t val) "bmdma: write (size %d) 
0x%"PRIx64" : 0x%02"PRIx64
+sii3112_bmdma_read(int size, uint64_t addr, uint64_t val) "read (size %d) 
0x%"PRIx64" : 0x%02"PRIx64
+sii3112_bmdma_write(int size, uint64_t addr, uint64_t val) "write (size %d) 
0x%"PRIx64" : 0x%02"PRIx64
+sii3112_read(int size, uint64_t addr, uint64_t val) "read (size %d) 0x%"PRIx64" : 
0x%02"PRIx64
+sii3112_write(int size, uint64_t addr, uint64_t val) "write (size %d) 0x%"PRIx64" : 
0x%02"PRIx64
  sii3112_set_irq(int channel, int level) "channel %d level %d"
# via.c 

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
reply via email to
[Prev in Thread] Current Thread [Next in Thread]