memory: unify ioport registration
v3 cover memory: unify ioport registration
This is the third version of patch series about ioport registration.
For the moment some part of QEMU use register_ioport* functions to register ioport. These functions don't permit the logging of range with Memory Listener.
Modifications between V1 and V2:
- Remove the use of get_system_io. Instead of use isa and pci IO address space.
- Avoid allocation of PortioList. Use the different device structure.
- Still remove register_ioport* (hw/dma.c, hw/apm.c, hw/acpi_piix4.c).
- Use MemoryRegion when we have only a range of ioport.
- For some functions, add IO address space as argument.
- Add isa_address_space_io function
Modifications between V2 and V2:
- Remove some register_ioport_* on hw/vt82c686.c.
- Split smb ioport part in new patch.
- Still replace MemoryRegion when we have only a range of ioport.
- Fix read/write ioports prototype to be compliant with memory callback.
Julien Grall (8):
isa: add isa_address_space_io
hw/acpi_piix4.c: replace register_ioport*
hw/cirrus_vga.c: replace register_ioport*
hw/serial.c: replace register_ioport*
hw/pc.c: replace register_ioport*
hw/dma.c: replace register_ioport*
hw/apm.c: replace register_ioport*
smb: replace_register_ioport*
hw/acpi_piix4.c | 130 ++++++++++++++++++++++++++++++++++++++++++++--------- hw/apm.c | 24 ++++++++-- hw/apm.h | 5 ++- hw/cirrus_vga.c | 38 +++++++++------- hw/dma.c | 84 ++++++++++++++++++++++++---------- hw/isa-bus.c | 5 ++ hw/isa.h | 1 + hw/mips_mipssim.c | 3 +- hw/pc.c | 58 +++++++++++++++++------ hw/pc.h | 2 +- hw/pm_smbus.c | 7 ++- hw/pm_smbus.h | 6 ++- hw/serial.c | 8 ++- hw/vt82c686.c | 20 +++++++- 14 files changed, 295 insertions(+), 96 deletions(-)
Signature · 1 line
Julien Grall
PATCH v3 4/8 hw/serial.c: replace register_ioport*
This patch replaces all register_ioport* with a MemoryRegion. It permits to use the new Memory stuff like listener.
For more flexibility, the IO address space is passed as an argument.
--- hw/mips_mipssim.c | 3 ++- hw/pc.h | 2 +- hw/serial.c | 8 +++++--- 3 files changed, 8 insertions(+), 5 deletions(-)
hw/mips_mipssim.c +2 -1
@@ -212,7 +212,8 @@ mips_mipssim_init (ram_addr_t ram_size,/* A single 16450 sits at offset 0x3f8. It is attached to MIPS CPU INT2, which is interrupt 4. */ if (serial_hds[0 ]) -serial_init (0x3f8 , env->irq[4 ],115200 , serial_hds[0 ]); +serial_init (0x3f8 , env->irq[4 ],115200 , serial_hds[0 ], +get_system_io ());if (nd_table[0 ].vlan)/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
hw/pc.h +1 -1
@@ -15,7 +15,7 @@/* serial.c */ SerialState *serial_init (int base,qemu_irq irq,int baudbase, -CharDriverState *chr); +CharDriverState *chr,MemoryRegion *system_io); SerialState *serial_mm_init(MemoryRegion *address_space, target_phys_addr_t base, int it_shift, qemu_irq irq, int baudbase,
hw/serial.c +5 -3
@@ -28,6 +28,7 @@#include "pc.h" #include "qemu-timer.h" #include "sysemu.h" +#include "exec-memory.h" //#define DEBUG_SERIAL
@@ -810,7 +811,7 @@ static const VMStateDescription vmstate_isa_serial = { };SerialState *serial_init (int base,qemu_irq irq,int baudbase, -CharDriverState *chr) +CharDriverState *chr,MemoryRegion *system_io) {SerialState *s;
@@ -823,8 +824,9 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,vmstate_register (NULL , base, &vmstate_serial, s); -register_ioport_write (base,8 ,1 , serial_ioport_write, s); -register_ioport_read (base,8 ,1 , serial_ioport_read, s); +memory_region_init_io (&s->io, &serial_io_ops, s,"serial" ,8 ); +memory_region_add_subregion (system_io, base, &s->io); +return s; }
Signature · 1 line
Julien Grall
PATCH v3 5/8 hw/pc.c: replace register_ioport*
In reply to Julien GrallThis patch replaces all register_ioport* with portio_* or isa_register_portio_list. It permits to use the new Memory stuff like listener.
--- hw/pc.c | 58 +++++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 43 insertions(+), 15 deletions(-)
hw/pc.c +43 -15
@@ -98,7 +98,8 @@ void gsi_handler(void *opaque, int n, int level)qemu_set_irq (s->ioapic_irq[n], level); } -static void ioport80_write (void *opaque,uint32_t addr,uint32_t data) +static void ioport80_write (void *opaque,target_phys_addr_t addr, +uint64_t data,unsigned size) { }
@@ -116,7 +117,8 @@ void cpu_set_ferr(CPUX86State *s)qemu_irq_raise (ferr_irq); } -static void ioportF0_write (void *opaque,uint32_t addr,uint32_t data) +static void ioportF0_write (void *opaque,target_phys_addr_t addr, +uint64_t data,unsigned size) {qemu_irq_lower (ferr_irq); }
@@ -592,6 +594,17 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)return index; } +static const MemoryRegionPortio bochs_bios_portio_list[] = { + {0x400 ,2 ,2 , .write = bochs_bios_write, },/* 0x400 */ + {0x402 ,2 ,1 , .write = bochs_bios_write, },/* 0x402 */ + {0x500 ,1 ,1 , .write = bochs_bios_write, },/* 0x500 */ + {0x501 ,1 ,1 , .write = bochs_bios_write, },/* 0x501 */ + {0x501 ,2 ,2 , .write = bochs_bios_write, },/* 0x501 */ + {0x503 ,1 ,1 , .write = bochs_bios_write, },/* 0x503 */ + {0x8900 ,1 ,1 , .write = bochs_bios_write, },/* 0x8900 */ +PORTIO_END_OF_LIST (), +}; +static void *bochs_bios_init (void ) {void *fw_cfg;
@@ -599,18 +612,11 @@ static void *bochs_bios_init(void)size_t smbios_len;uint64_t *numa_fw_cfg;int i, j; +PortioList *bochs_bios_port_list =g_new (PortioList,1 ); -register_ioport_write (0x400 ,1 ,2 , bochs_bios_write,NULL ); -register_ioport_write (0x401 ,1 ,2 , bochs_bios_write,NULL ); -register_ioport_write (0x402 ,1 ,1 , bochs_bios_write,NULL ); -register_ioport_write (0x403 ,1 ,1 , bochs_bios_write,NULL ); -register_ioport_write (0x8900 ,1 ,1 , bochs_bios_write,NULL ); - -register_ioport_write (0x501 ,1 ,1 , bochs_bios_write,NULL ); -register_ioport_write (0x501 ,1 ,2 , bochs_bios_write,NULL ); -register_ioport_write (0x502 ,1 ,2 , bochs_bios_write,NULL ); -register_ioport_write (0x500 ,1 ,1 , bochs_bios_write,NULL ); -register_ioport_write (0x503 ,1 ,1 , bochs_bios_write,NULL ); +portio_list_init (bochs_bios_port_list, bochs_bios_portio_list, +NULL ,"bosch-bios" ); +portio_list_add (bochs_bios_port_list,get_system_io (),0x0 ); fw_cfg =fw_cfg_init (BIOS_CFG_IOPORT ,BIOS_CFG_IOPORT +1 ,0 ,0 );
@@ -1077,6 +1083,24 @@ static void cpu_request_exit(void *opaque, int irq, int level) } } +static const MemoryRegionOps ioport80_io_ops = { + .write = ioport80_write, + .endianness =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; + +static const MemoryRegionOps ioportF0_io_ops = { + .write = ioportF0_write, + .endianness =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; + void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, ISADevice **floppy,
@@ -1091,10 +1115,14 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,qemu_irq *a20_line;ISADevice *i8042, *port92, *vmmouse, *pit;qemu_irq *cpu_exit_irq; +MemoryRegion *ioport80_io =g_new (MemoryRegion,1 ); +MemoryRegion *ioportF0_io =g_new (MemoryRegion,1 ); -register_ioport_write (0x80 ,1 ,1 , ioport80_write,NULL ); +memory_region_init_io (ioport80_io, &ioport80_io_ops,NULL ,"ioport80" ,1 ); +memory_region_add_subregion (isa_address_space_io (NULL ),0x80 , ioport80_io); -register_ioport_write (0xf0 ,1 ,1 , ioportF0_write,NULL ); +memory_region_init_io (ioportF0_io, &ioportF0_io_ops,NULL ,"ioportF0" ,1 ); +memory_region_add_subregion (isa_address_space_io (NULL ),0xf0 , ioportF0_io);/* * Check if an HPET shall be created.
Signature · 1 line
Julien Grall
PATCH v3 6/8 hw/dma.c: replace register_ioport*
In reply to Julien GrallThis patch replaces all register_ioport* be the new memory API functions. It permits to use the new Memory stuff like listener.
--- hw/dma.c | 84 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 60 insertions(+), 24 deletions(-)
hw/dma.c +60 -24
@@ -58,6 +58,8 @@ static struct dma_cont {int dshift;struct dma_regs regs[4 ];qemu_irq *cpu_request_exit; +MemoryRegion channel_io[8 ]; +MemoryRegion cont_io[8 ]; } dma_controllers[2 ];enum {
@@ -149,7 +151,7 @@ static inline int getff (struct dma_cont *d)return ff; } -static uint32_t read_chan (void *opaque,uint32_t nport) +static uint64_t read_chan (void *opaque,uint64_t nport,unsigned size) {struct dma_cont *d = opaque;int ichan, nreg, iport, ff, val, dir;
@@ -171,7 +173,8 @@ static uint32_t read_chan (void *opaque, uint32_t nport)return (val >> (d->dshift + (ff <<3 ))) &0xff ; } -static void write_chan (void *opaque,uint32_t nport,uint32_t data) +static void write_chan (void *opaque,target_phys_addr_t nport,uint64_t data, +unsigned size) {struct dma_cont *d = opaque;int iport, ichan, nreg;
@@ -189,7 +192,8 @@ static void write_chan (void *opaque, uint32_t nport, uint32_t data) } } -static void write_cont (void *opaque,uint32_t nport,uint32_t data) +static void write_cont (void *opaque,target_phys_addr_t nport,uint64_t data, +unsigned size) {struct dma_cont *d = opaque;int iport, ichan =0 ;
@@ -198,7 +202,7 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)switch (iport) {case 0x08 :/* command */ if ((data !=0 ) && (data &CMD_NOT_SUPPORTED )) { -dolog ("command %#x not supported\n" , data); +dolog ("command %#lx not supported\n" , data);return ; } d->command = data;
@@ -277,7 +281,8 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)#endif } -static uint32_t read_cont (void *opaque,uint32_t nport) +static uint64_t read_cont (void *opaque,target_phys_addr_t nport, +unsigned size) {struct dma_cont *d = opaque;int iport, val;
@@ -463,7 +468,7 @@ void DMA_schedule(int nchan)static void dma_reset (void *opaque) {struct dma_cont *d = opaque; -write_cont (d, (0x0d << d->dshift),0 ); +write_cont (d, (0x0d << d->dshift),0 ,1 ); }static int dma_phony_handler (void *opaque,int nchan,int dma_pos,int dma_len)
@@ -473,37 +478,68 @@ static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)return dma_pos; } + +static const MemoryRegionOps channel_io_ops = { + .read = read_chan, + .write = write_chan, + .endianness =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; + +/* IOport from page_base */ +static const MemoryRegionPortio page_portio_list[] = { + {0x01 ,3 ,1 , .write = write_page, .read = read_page, }, + {0x07 ,1 ,1 , .write = write_page, .read = read_page, }, +PORTIO_END_OF_LIST (), +}; + +/* IOport from pageh_base */ +static const MemoryRegionPortio pageh_portio_list[] = { + {0x01 ,3 ,1 , .write = write_pageh, .read = read_pageh, }, + {0x07 ,3 ,1 , .write = write_pageh, .read = read_pageh, }, +PORTIO_END_OF_LIST (), +}; + +static const MemoryRegionOps cont_io_ops = { + .read = read_cont, + .write = write_cont, + .endianness =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; +/* dshift = 0: 8 bit DMA, 1 = 16 bit DMA */ static void dma_init2 (struct dma_cont *d,int base,int dshift,int page_base,int pageh_base,qemu_irq *cpu_request_exit) { -static const int page_port_list[] = {0x1 ,0x2 ,0x3 ,0x7 };int i; d->dshift = dshift; d->cpu_request_exit = cpu_request_exit;for (i =0 ; i <8 ; i++) { -register_ioport_write (base + (i << dshift),1 ,1 , write_chan, d); -register_ioport_read (base + (i << dshift),1 ,1 , read_chan, d); +memory_region_init_io (&d->channel_io[i], &channel_io_ops, d, +"dma-chan" ,1 ); +memory_region_add_subregion (isa_address_space_io (NULL ), + base + (i << dshift), &d->channel_io[i]); } -for (i =0 ; i <ARRAY_SIZE (page_port_list); i++) { -register_ioport_write (page_base + page_port_list[i],1 ,1 , - write_page, d); -register_ioport_read (page_base + page_port_list[i],1 ,1 , - read_page, d); -if (pageh_base >=0 ) { -register_ioport_write (pageh_base + page_port_list[i],1 ,1 , - write_pageh, d); -register_ioport_read (pageh_base + page_port_list[i],1 ,1 , - read_pageh, d); - } + +isa_register_portio_list (NULL , page_base, page_portio_list, d, +"dma-page" ); +if (pageh_base >=0 ) { +isa_register_portio_list (NULL , pageh_base, pageh_portio_list, d, +"dma-pageh" ); }for (i =0 ; i <8 ; i++) { -register_ioport_write (base + ((i +8 ) << dshift),1 ,1 , - write_cont, d); -register_ioport_read (base + ((i +8 ) << dshift),1 ,1 , - read_cont, d); +memory_region_init_io (&d->cont_io[i], &cont_io_ops, d,"dma-cont" ,1 ); +memory_region_add_subregion (isa_address_space_io (NULL ), + base + ((i +8 ) << dshift), + &d->cont_io[i]); }qemu_register_reset (dma_reset, d);dma_reset (d);
Signature · 1 line
Julien Grall
PATCH v3 7/8 hw/apm.c: replace register_ioport*
In reply to Julien GrallThis patch replaces all register_ioport* by a MemorySection. It permits to use the new Memory stuff like listener.
Moreover, the PCI is added as an argument for apm_init, so we can register IO inside the pci IO address space.
--- hw/acpi_piix4.c | 2 +- hw/apm.c | 24 +++++++++++++++++++----- hw/apm.h | 5 ++++- hw/vt82c686.c | 2 +- 4 files changed, 25 insertions(+), 8 deletions(-)
hw/acpi_piix4.c +1 -1
@@ -356,7 +356,7 @@ static int piix4_pm_initfn(PCIDevice *dev) pci_conf[0x40 ] =0x01 ;/* PM io base read only bit */ /* APM */ -apm_init (&s->apm, apm_ctrl_changed, s); +apm_init (dev, &s->apm, apm_ctrl_changed, s);memory_region_init_io (&s->acpi_io, &acpi_io_ops, s,"piix4-acpi" ,4 ); memory_region_add_subregion(pci_address_space_io(dev), ACPI_DBG_IO_ADDR,
hw/apm.c +19 -5
@@ -22,6 +22,7 @@#include "apm.h" #include "hw.h" +#include "pci.h" //#define DEBUG
@@ -35,7 +36,8 @@#define APM_CNT_IOPORT 0xb2#define APM_STS_IOPORT 0xb3 -static void apm_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void apm_ioport_writeb (void *opaque,target_phys_addr_t addr, +uint64_t val,unsigned size) {APMState *apm = opaque; addr &=1 ;
@@ -51,7 +53,8 @@ static void apm_ioport_writeb(void *opaque, uint32_t addr, uint32_t val) } } -static uint32_t apm_ioport_readb (void *opaque,uint32_t addr) +static uint64_t apm_ioport_readb (void *opaque,target_phys_addr_t addr, +unsigned size) {APMState *apm = opaque;uint32_t val;
@@ -78,12 +81,23 @@ const VMStateDescription vmstate_apm = { } }; -void apm_init (APMState *apm,apm_ctrl_changed_t callback,void *arg) +static const MemoryRegionOps apm_ops = { + .read = apm_ioport_readb, + .write = apm_ioport_writeb, + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; + +void apm_init (PCIDevice *dev,APMState *apm,apm_ctrl_changed_t callback, +void *arg) { apm->callback = callback; apm->arg = arg;/* ioport 0xb2, 0xb3 */ -register_ioport_write (APM_CNT_IOPORT ,2 ,1 , apm_ioport_writeb, apm); -register_ioport_read (APM_CNT_IOPORT ,2 ,1 , apm_ioport_readb, apm); +memory_region_init_io (&apm->io, &apm_ops, apm,"apm-io" ,2 ); +memory_region_add_subregion (pci_address_space_io (dev),APM_CNT_IOPORT , + &apm->io); }
hw/apm.h +4 -1
@@ -4,6 +4,7 @@#include <stdint.h> #include "qemu-common.h" #include "hw.h" +#include "memory.h" typedef void (*apm_ctrl_changed_t )(uint32_t val,void *arg);
@@ -13,9 +14,11 @@ typedef struct APMState {apm_ctrl_changed_t callback;void *arg; +MemoryRegion io; } APMState; -void apm_init (APMState *s,apm_ctrl_changed_t callback,void *arg); +void apm_init (PCIDevice *dev,APMState *s,apm_ctrl_changed_t callback, +void *arg);extern const VMStateDescription vmstate_apm;
hw/vt82c686.c +1 -1
@@ -427,7 +427,7 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)register_ioport_write (s->smb_io_base,0xf ,1 , smb_ioport_writeb, &s->smb);register_ioport_read (s->smb_io_base,0xf ,1 , smb_ioport_readb, &s->smb); -apm_init (&s->apm,NULL , s); +apm_init (dev, &s->apm,NULL , s);acpi_pm_tmr_init (&s->ar, pm_tmr_timer);acpi_pm1_cnt_init (&s->ar);
Signature · 1 line
Julien Grall
PATCH v3 8/8 smb: replace_register_ioport*
In reply to Julien GrallThis patch fix smb_ioport_* to be compliant with read/write memory callback. Moreover it replaces all register_ioport* which use theses functions by the new Memory API.
--- hw/acpi_piix4.c | 16 ++++++++++++++-- hw/pm_smbus.c | 7 ++++--- hw/pm_smbus.h | 6 ++++-- hw/vt82c686.c | 18 ++++++++++++++++-- 4 files changed, 38 insertions(+), 9 deletions(-)
hw/acpi_piix4.c +14 -2
@@ -333,6 +333,16 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) } +static const MemoryRegionOps smb_io_ops = { + .read = smb_ioport_readb, + .write = smb_ioport_writeb, + .endianness =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; +static const MemoryRegionOps acpi_io_ops = { .write = acpi_dbg_writel, .endianness =DEVICE_NATIVE_ENDIAN ,
@@ -373,8 +383,10 @@ static int piix4_pm_initfn(PCIDevice *dev) pci_conf[0x90 ] = s->smb_io_base |1 ; pci_conf[0x91 ] = s->smb_io_base >>8 ; pci_conf[0xd2 ] =0x09 ; -register_ioport_write (s->smb_io_base,64 ,1 , smb_ioport_writeb, &s->smb); -register_ioport_read (s->smb_io_base,64 ,1 , smb_ioport_readb, &s->smb); + +memory_region_init_io (&s->smb_io, &smb_io_ops, &s->smb,"piix4-smb" ,64 ); +memory_region_add_subregion (pci_address_space_io (dev), s->smb_io_base, + &s->smb_io);acpi_pm_tmr_init (&s->ar, pm_tmr_timer);acpi_gpe_init (&s->ar,GPE_LEN );
hw/pm_smbus.c +4 -3
@@ -94,7 +94,8 @@ static void smb_transaction(PMSMBus *s) s->smb_stat |=0x04 ; } -void smb_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +void smb_ioport_writeb (void *opaque,target_phys_addr_t addr,uint64_t val, +unsigned size) {PMSMBus *s = opaque; addr &=0x3f ;
@@ -131,10 +132,10 @@ void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val) } } -uint32_t smb_ioport_readb (void *opaque,uint32_t addr) +uint64_t smb_ioport_readb (void *opaque,target_phys_addr_t addr,unsigned size) {PMSMBus *s = opaque; -uint32_t val; +uint64_t val; addr &=0x3f ;switch (addr) {
hw/pm_smbus.h +4 -2
@@ -15,7 +15,9 @@ typedef struct PMSMBus { } PMSMBus;void pm_smbus_init (DeviceState *parent,PMSMBus *smb); -void smb_ioport_writeb (void *opaque,uint32_t addr,uint32_t val); -uint32_t smb_ioport_readb (void *opaque,uint32_t addr); +void smb_ioport_writeb (void *opaque,target_phys_addr_t addr,uint64_t val, +unsigned size); +uint64_t smb_ioport_readb (void *opaque,target_phys_addr_t addr, +unsigned size);#endif /* !PM_SMBUS_H */
hw/vt82c686.c +16 -2
@@ -163,6 +163,7 @@ typedef struct VT686PMState {APMState apm;PMSMBus smb;uint32_t smb_io_base; +MemoryRegion smb_io; } VT686PMState; typedef struct VT686AC97State {
@@ -405,6 +406,16 @@ static TypeInfo via_mc97_info = { .class_init = via_mc97_class_init, }; +static const MemoryRegionOps smb_io_ops = { + .read = smb_ioport_readb, + .write = smb_ioport_writeb, + .endianess =DEVICE_NATIVE_ENDIAN , + .impl = { + .min_access_size =1 , + .max_access_size =1 , + }, +}; +/* vt82c686 pm init */ static int vt82c686b_pm_initfn (PCIDevice *dev) {
@@ -424,8 +435,11 @@ static int vt82c686b_pm_initfn(PCIDevice *dev) pci_conf[0x90 ] = s->smb_io_base |1 ; pci_conf[0x91 ] = s->smb_io_base >>8 ; pci_conf[0xd2 ] =0x90 ; -register_ioport_write (s->smb_io_base,0xf ,1 , smb_ioport_writeb, &s->smb); -register_ioport_read (s->smb_io_base,0xf ,1 , smb_ioport_readb, &s->smb); + +memory_region_init_io (&s->smb_io, &smb_io_ops, &s->smb,"vt82c686b-smb" , +0xf ); +memory_region_add_subregion (pci_address_space_io (dev), s->smb_io_base, + &s->smb_io);apm_init (dev, &s->apm,NULL , s);
Signature · 1 line
Julien Grall