diff mbox series

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

Message ID 20230422150728.176512-13-shentey@gmail.com (mailing list archive)
State New, archived
Headers show
Series Clean up PCI IDE device models | expand

Commit Message

Bernhard Beschow April 22, 2023, 3:07 p.m. UTC
Allows to unexport bmdma_addr_ioport_ops and models TYPE_SII3112_PCI as a
standard-compliant PCI IDE device.

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(-)

Comments

Mark Cave-Ayland April 26, 2023, 11:44 a.m. UTC | #1
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.
Bernhard Beschow April 26, 2023, 8:26 p.m. UTC | #2
Am 26. April 2023 11:44:29 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>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.

Will do. I'd also split this patch into two.

Best regards,
Bernhard

>
>> 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.
diff mbox series

Patch

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