memory: remove old_portio usage
cover memory: remove old_portio usage
These proposed patches aim at removing the .old_portio member of MemoryRegionOps structure, and replacing their usage by .read/.write handlers.
That way, faked I/O address space can be removed from architectures which don't have it (MIPS, PowerPC...), and commits like a178274efabcbbc5d44805b51def874e47051325 ("PPC: pseries: Remove hack for PIO window") can be reapplied.
Changes since RFC:
- fixed ppc/oldworld, ppc/newworld and sun4u machines
- removed bochs port changes, as they have been committed
Gerd Hoffmann (1):
uhci: stop using portio lists
Hervé Poussineau (9):
ppc/newworld: add ISA bus, required by VGA card ppc/oldworld: add ISA bus, required by VGA card
sun4u: create VGA card after ISA bus
xen_platform: do not use old_portio-style callbacks
acpi-piix4: do not use old_portio-style callbacks vga/qxl: do not use portio_list_init/portio_list_add
isa: use memory regions instead of portio_list_* functions
ioport: remove now useless portio_list_* functions
memory: remove old_portio-style callbacks support
hw/acpi_piix4.c | 91 ++++++++++++++++------------------- hw/isa-bus.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++-- hw/isa.h | 2 +- hw/ppc_newworld.c | 1 + hw/ppc_oldworld.c | 1 + hw/qxl.c | 4 +- hw/sun4u.c | 3 +- hw/usb/hcd-uhci.c | 106 ++++++++++++----------------------------- hw/vga.c | 8 +--- hw/xen_platform.c | 21 ++++---- include/exec/ioport.h | 19 -------- include/exec/memory.h | 4 -- ioport.c | 121 ---------------------------------------------- memory.c | 44 ----------------- trace-events | 2 - 15 files changed, 212 insertions(+), 342 deletions(-)
Signature · 1 line
1.7.10.4
PATCH 1/10 ppc/newworld: add ISA bus, required by VGA card
Signed-off-by Hervé Poussineau <[email protected]>
--- hw/ppc_newworld.c | 1 + 1 file changed, 1 insertion(+)
hw/ppc_newworld.c +1 -0
@@ -274,6 +274,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) }/* Register 8 MB of ISA IO space */ +isa_bus_new (NULL ,get_system_io ());isa_mmio_init (0xf2000000 ,0x00800000 );/* UniN init */
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH 01/10] ppc/newworld: add ISA bus, required by VGA card
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 10 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/ppc_newworld.c | 1 + 1 file changed, 1 insertion(+)hw/ppc_newworld.c +0
-0
hw/ppc_newworld.c static void ppc_core99_init(QEMUMachineInitArgs *args) · line 280
Quoted 7 lines
hw/ppc_newworld.c +1
-0@@ -274,6 +274,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) }/* Register 8 MB of ISA IO space */ +isa_bus_new (NULL ,get_system_io ());isa_mmio_init (0xf2000000 ,0x00800000 );/* UniN init */
I understand that a bus will be necessary for the ISA I/O ports, but I do not like the bus added to the machine like this when at the same time we are trying to QOM'ify the chipsets for IDE.
Where does this device logically sit? On UniNorth chipset?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
PATCH 2/10 ppc/oldworld: add ISA bus, required by VGA card
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/ppc_oldworld.c | 1 + 1 file changed, 1 insertion(+)
hw/ppc_oldworld.c +1 -0
@@ -223,6 +223,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args) }/* Register 2 MB of ISA IO space */ +isa_bus_new (NULL ,get_system_io ());isa_mmio_init (0xfe000000 ,0x00200000 );/* XXX: we register only 1 output pin for heathrow PIC */
Signature · 1 line
1.7.10.4
PATCH 3/10 uhci: stop using portio lists
In reply to Hervé PoussineauFrom: Gerd Hoffmann <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>
Tested-by Hervé Poussineau <[email protected]>
--- hw/usb/hcd-uhci.c | 106 +++++++++++++++-------------------------------------- trace-events | 2 - 2 files changed, 30 insertions(+), 78 deletions(-)
hw/usb/hcd-uhci.c +30 -76
@@ -455,40 +455,11 @@ static const VMStateDescription vmstate_uhci = { } }; -static void uhci_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void uhci_port_write (void *opaque,hwaddr addr, +uint64_t val,unsigned size) {UHCIState *s = opaque; - addr &=0x1f ; -switch (addr) { -case 0x0c : - s->sof_timing = val; -break ; - } -} - -static uint32_t uhci_ioport_readb (void *opaque,uint32_t addr) -{ -UHCIState *s = opaque; -uint32_t val; - - addr &=0x1f ; -switch (addr) { -case 0x0c : - val = s->sof_timing; -break ; -default : - val =0xff ; -break ; - } -return val; -} - -static void uhci_ioport_writew (void *opaque,uint32_t addr,uint32_t val) -{ -UHCIState *s = opaque; - - addr &=0x1f ;trace_usb_uhci_mmio_writew (addr, val);switch (addr) {
@@ -537,6 +508,17 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)if (s->status &UHCI_STS_HCHALTED ) s->frnum = val &0x7ff ;break ; +case 0x08 : + s->fl_base_addr &=0xffff0000 ; + s->fl_base_addr |= val & ~0xfff ; +break ; +case 0x0a : + s->fl_base_addr &=0x0000ffff ; + s->fl_base_addr |= (val <<16 ); +break ; +case 0x0c : + s->sof_timing = val &0xff ; +break ; case 0x10 ... 0x1f: {UHCIPort *port;
@@ -568,12 +550,11 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val) } } -static uint32_t uhci_ioport_readw (void *opaque,uint32_t addr) +static uint64_t uhci_port_read (void *opaque,hwaddr addr,unsigned size) {UHCIState *s = opaque;uint32_t val; - addr &=0x1f ;switch (addr) {case 0x00 : val = s->cmd;
@@ -587,6 +568,15 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)case 0x06 : val = s->frnum;break ; +case 0x08 : + val = s->fl_base_addr &0xffff ; +break ; +case 0x0a : + val = (s->fl_base_addr >>16 ) &0xffff ; +break ; +case 0x0c : + val = s->sof_timing; +break ; case 0x10 ... 0x1f: {UHCIPort *port;
@@ -609,38 +599,6 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)return val; } -static void uhci_ioport_writel (void *opaque,uint32_t addr,uint32_t val) -{ -UHCIState *s = opaque; - - addr &=0x1f ; -trace_usb_uhci_mmio_writel (addr, val); - -switch (addr) { -case 0x08 : - s->fl_base_addr = val & ~0xfff ; -break ; - } -} - -static uint32_t uhci_ioport_readl (void *opaque,uint32_t addr) -{ -UHCIState *s = opaque; -uint32_t val; - - addr &=0x1f ; -switch (addr) { -case 0x08 : - val = s->fl_base_addr; -break ; -default : - val =0xffffffff ; -break ; - } -trace_usb_uhci_mmio_readl (addr, val); -return val; -} -/* signal resume if controller suspended */ static void uhci_resume (void *opaque) {
@@ -1213,18 +1171,14 @@ static void uhci_frame_timer(void *opaque)qemu_mod_timer (s->frame_timer, s->expire_time); } -static const MemoryRegionPortio uhci_portio[] = { - {0 ,32 ,2 , .write = uhci_ioport_writew, }, - {0 ,32 ,2 , .read = uhci_ioport_readw, }, - {0 ,32 ,4 , .write = uhci_ioport_writel, }, - {0 ,32 ,4 , .read = uhci_ioport_readl, }, - {0 ,32 ,1 , .write = uhci_ioport_writeb, }, - {0 ,32 ,1 , .read = uhci_ioport_readb, }, -PORTIO_END_OF_LIST () -}; -static const MemoryRegionOps uhci_ioport_ops = { - .old_portio = uhci_portio, + .read = uhci_port_read, + .write = uhci_port_write, + .valid.min_access_size =1 , + .valid.max_access_size =4 , + .impl.min_access_size =2 , + .impl.max_access_size =2 , + .endianness =DEVICE_LITTLE_ENDIAN , };static USBPortOps uhci_port_ops = {
trace-events +0 -2
@@ -307,8 +307,6 @@ usb_uhci_frame_loop_stop_idle(void) "" usb_uhci_frame_loop_continue(void) "" usb_uhci_mmio_readw(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%04x" usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%04x" -usb_uhci_mmio_readl(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%08x" -usb_uhci_mmio_writel(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%08x" usb_uhci_queue_add(uint32_t token) "token 0x%x" usb_uhci_queue_del(uint32_t token, const char *reason) "token 0x%x: %s" usb_uhci_packet_add(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH 03/10] uhci: stop using portio lists
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 4 lines
From: Gerd Hoffmann <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>Tested-by Hervé Poussineau <[email protected]>
This is missing your Sob, but I believe it was merged through Gerd's USB queue already.
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 03/10] uhci: stop using portio lists
Quoted 8 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 4 lines
From: Gerd Hoffmann <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>Tested-by Hervé Poussineau <[email protected]>This is missing your Sob, but I believe it was merged through Gerd's USB queue already.
Yes, it's in already via usb queue, git will drop it the next time you rebase the patchset.
cheers,
Gerd
PATCH 4/10 sun4u: create VGA card after ISA bus
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/sun4u.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
hw/sun4u.c +2 -1
@@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem, ivec_irqs =qemu_allocate_irqs (cpu_set_ivec_irq, cpu,IVEC_MAX ); pci_bus =pci_apb_init (APB_SPECIAL_BASE ,APB_MEM_BASE , ivec_irqs, &pci_bus2, &pci_bus3, &pbm_irqs); -pci_vga_init (pci_bus);// XXX Should be pci_bus3 isa_bus =pci_ebus_init (pci_bus,-1 , pbm_irqs); +pci_vga_init (pci_bus); + i =0 ;if (hwdef->console_serial_base) { serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 4 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/sun4u.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10, which switches to isa_register_portio_list().
Andreas
Quoted 20 lines
hw/sun4u.c +2
-1@@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem, ivec_irqs =qemu_allocate_irqs (cpu_set_ivec_irq, cpu,IVEC_MAX ); pci_bus =pci_apb_init (APB_SPECIAL_BASE ,APB_MEM_BASE , ivec_irqs, &pci_bus2, &pci_bus3, &pbm_irqs); -pci_vga_init (pci_bus);// XXX Should be pci_bus3 isa_bus =pci_ebus_init (pci_bus,-1 , pbm_irqs); +pci_vga_init (pci_bus); + i =0 ;if (hwdef->console_serial_base) { serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus
Quoted 8 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 4 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/sun4u.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10, which switches to
isa_register_portio_list().
Indeed.
ISA bus is created in pci_ebus_init(). VGA card registers some ISA I/O ports, so an ISA bus must be present.
Indeed.
Quoted 24 lines
Andreas
Quoted 19 lines
hw/sun4u.c +2
-1@@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem, ivec_irqs =qemu_allocate_irqs (cpu_set_ivec_irq, cpu,IVEC_MAX ); pci_bus =pci_apb_init (APB_SPECIAL_BASE ,APB_MEM_BASE , ivec_irqs, &pci_bus2, &pci_bus3, &pbm_irqs); -pci_vga_init (pci_bus);// XXX Should be pci_bus3 isa_bus =pci_ebus_init (pci_bus,-1 , pbm_irqs); +pci_vga_init (pci_bus); + i =0 ;if (hwdef->console_serial_base) { serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
Re: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus
Quoted 10 lines · Hervé Poussineau
Quoted 8 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 5 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/sun4u.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 7 lines
Quoted 2 lines
Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10, which switches to
isa_register_portio_list().Indeed.
ISA bus is created inpci_ebus_init(). VGA card registers some ISA I/O ports, so an ISA bus must be present.
OK by me.
Quoted 29 lines
Indeed.
Quoted 24 lines
Andreas
Quoted 19 lines
hw/sun4u.c +1
-1@@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem, ivec_irqs =qemu_allocate_irqs (cpu_set_ivec_irq, cpu,IVEC_MAX ); pci_bus =pci_apb_init (APB_SPECIAL_BASE ,APB_MEM_BASE , ivec_irqs, &pci_bus2, &pci_bus3, &pbm_irqs); -pci_vga_init (pci_bus);// XXX Should be pci_bus3 isa_bus =pci_ebus_init (pci_bus,-1 , pbm_irqs); +pci_vga_init (pci_bus); + i =0 ;if (hwdef->console_serial_base) { serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
Andreas Färber
AppliedRe: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus
Am 12.01.2013 12:42, schrieb Blue Swirl:
Quoted 15 lines · Hervé Poussineau
Quoted 14 lines
Quoted 10 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 5 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/sun4u.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10, which switches to
isa_register_portio_list().
Quoted 5 lines
Quoted 3 lines
Indeed.
ISA bus is created inpci_ebus_init(). VGA card registers some ISA I/O ports, so an ISA bus must be present.OK by me.
Thanks, applied to memory-ioport branch (with an additional explanatory sentence in the commit message):
https://github.com/afaerber/qemu-cpu/commits/memory-ioport
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
PATCH 5/10 xen_platform: do not use old_portio-style callbacks
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/xen_platform.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
hw/xen_platform.c +10 -11
@@ -279,7 +279,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)/* Xen Platform PCI Device */ -static uint32_t xen_platform_ioport_readb (void *opaque,uint32_t addr) +static uint64_t xen_platform_ioport_readb (void *opaque,hwaddr addr, +unsigned int size) {if (addr ==0 ) {return platform_fixed_ioport_readb (opaque,0 );
@@ -288,30 +289,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) } } -static void xen_platform_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void xen_platform_ioport_writeb (void *opaque,hwaddr addr, +uint64_t val,unsigned int size) {PCIXenPlatformState *s = opaque;switch (addr) {case 0 :/* Platform flags */ -platform_fixed_ioport_writeb (opaque,0 , val); +platform_fixed_ioport_writeb (opaque,0 , (uint32_t )val);break ;case 8 : -log_writeb (s, val); +log_writeb (s, (uint32_t )val);break ;default :break ; } } -static MemoryRegionPortio xen_pci_portio[] = { - {0 ,0x100 ,1 , .read = xen_platform_ioport_readb, }, - {0 ,0x100 ,1 , .write = xen_platform_ioport_writeb, }, -PORTIO_END_OF_LIST () -}; -static const MemoryRegionOps xen_pci_io_ops = { - .old_portio = xen_pci_portio, + .read = xen_platform_ioport_readb, + .write = xen_platform_ioport_writeb, + .impl.min_access_size =1 , + .impl.max_access_size =1 , };static void platform_ioport_bar_setup (PCIXenPlatformState *d)
Signature · 7 lines
1.7.10.4 _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel
Andreas Färber
AppliedRe: [Qemu-devel] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 48 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/xen_platform.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)hw/xen_platform.c +6
-9@@ -279,7 +279,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)/* Xen Platform PCI Device */ -static uint32_t xen_platform_ioport_readb (void *opaque,uint32_t addr) +static uint64_t xen_platform_ioport_readb (void *opaque,hwaddr addr, +unsigned int size) {if (addr ==0 ) {return platform_fixed_ioport_readb (opaque,0 );@@ -288,30 +289,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) } } -static void xen_platform_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void xen_platform_ioport_writeb (void *opaque,hwaddr addr, +uint64_t val,unsigned int size) {PCIXenPlatformState *s = opaque;switch (addr) {case 0 :/* Platform flags */ -platform_fixed_ioport_writeb (opaque,0 , val); +platform_fixed_ioport_writeb (opaque,0 , (uint32_t )val);break ;case 8 : -log_writeb (s, val); +log_writeb (s, (uint32_t )val);break ;default :break ; } } -static MemoryRegionPortio xen_pci_portio[] = { - {0 ,0x100 ,1 , .read = xen_platform_ioport_readb, }, - {0 ,0x100 ,1 , .write = xen_platform_ioport_writeb, }, -PORTIO_END_OF_LIST () -};
hw/xen_platform.c static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) · line 316
Quoted 10 lines
hw/xen_platform.c +4
-2-static const MemoryRegionOps xen_pci_io_ops = { - .old_portio = xen_pci_portio, + .read = xen_platform_ioport_readb, + .write = xen_platform_ioport_writeb, + .impl.min_access_size =1 , + .impl.max_access_size =1 , };static void platform_ioport_bar_setup (PCIXenPlatformState *d)
Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioport
Is there some kind of guidance in which cases to use .impl vs. .valid for the access sizes in these conversions?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks
Stefano,
Am 12.01.2013 17:06, schrieb Andreas Färber:
Quoted 58 lines
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 57 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/xen_platform.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)hw/xen_platform.c +10
-11@@ -279,7 +279,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)/* Xen Platform PCI Device */ -static uint32_t xen_platform_ioport_readb (void *opaque,uint32_t addr) +static uint64_t xen_platform_ioport_readb (void *opaque,hwaddr addr, +unsigned int size) {if (addr ==0 ) {return platform_fixed_ioport_readb (opaque,0 );@@ -288,30 +289,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) } } -static void xen_platform_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void xen_platform_ioport_writeb (void *opaque,hwaddr addr, +uint64_t val,unsigned int size) {PCIXenPlatformState *s = opaque;switch (addr) {case 0 :/* Platform flags */ -platform_fixed_ioport_writeb (opaque,0 , val); +platform_fixed_ioport_writeb (opaque,0 , (uint32_t )val);break ;case 8 : -log_writeb (s, val); +log_writeb (s, (uint32_t )val);break ;default :break ; } } -static MemoryRegionPortio xen_pci_portio[] = { - {0 ,0x100 ,1 , .read = xen_platform_ioport_readb, }, - {0 ,0x100 ,1 , .write = xen_platform_ioport_writeb, }, -PORTIO_END_OF_LIST () -}; -static const MemoryRegionOps xen_pci_io_ops = { - .old_portio = xen_pci_portio, + .read = xen_platform_ioport_readb, + .write = xen_platform_ioport_writeb, + .impl.min_access_size =1 , + .impl.max_access_size =1 , };
Quoted 4 lines
hw/xen_platform.c static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) · line 316
Quoted 1 line
hw/xen_platform.c +0
-0static void platform_ioport_bar_setup (PCIXenPlatformState *d)Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioport
If you want to ack or object, now would be the time. :)
https://github.com/afaerber/qemu-cpu/commit/016861a22283da3931c4b343e6d2df0dc123b6ef
Cheers,
Andreas
Quoted 5 lines
Is there some kind of guidance in which cases to use .impl vs. .valid for the access sizes in these conversions?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks
Quoted 61 lines · Andreas Färber
Stefano,
Am 12.01.2013 17:06, schrieb Andreas Färber:
Quoted 58 lines
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 57 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/xen_platform.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)hw/xen_platform.c +10
-11@@ -279,7 +279,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)/* Xen Platform PCI Device */ -static uint32_t xen_platform_ioport_readb (void *opaque,uint32_t addr) +static uint64_t xen_platform_ioport_readb (void *opaque,hwaddr addr, +unsigned int size) {if (addr ==0 ) {return platform_fixed_ioport_readb (opaque,0 );@@ -288,30 +289,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) } } -static void xen_platform_ioport_writeb (void *opaque,uint32_t addr,uint32_t val) +static void xen_platform_ioport_writeb (void *opaque,hwaddr addr, +uint64_t val,unsigned int size) {PCIXenPlatformState *s = opaque;switch (addr) {case 0 :/* Platform flags */ -platform_fixed_ioport_writeb (opaque,0 , val); +platform_fixed_ioport_writeb (opaque,0 , (uint32_t )val);break ;case 8 : -log_writeb (s, val); +log_writeb (s, (uint32_t )val);break ;default :break ; } } -static MemoryRegionPortio xen_pci_portio[] = { - {0 ,0x100 ,1 , .read = xen_platform_ioport_readb, }, - {0 ,0x100 ,1 , .write = xen_platform_ioport_writeb, }, -PORTIO_END_OF_LIST () -}; -static const MemoryRegionOps xen_pci_io_ops = { - .old_portio = xen_pci_portio, + .read = xen_platform_ioport_readb, + .write = xen_platform_ioport_writeb, + .impl.min_access_size =1 , + .impl.max_access_size =1 , };
Quoted 7 lines
Quoted 4 lines
hw/xen_platform.c static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) · line 316
Quoted 1 line
hw/xen_platform.c +0
-0static void platform_ioport_bar_setup (PCIXenPlatformState *d)Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioportIf you want to ack or object, now would be the time. :)
https://github.com/afaerber/qemu-cpu/commit/016861a22283da3931c4b343e6d2df0dc123b6ef
That patch is also fine by me. Thanks for the heads up!
PATCH 6/10 acpi-piix4: do not use old_portio-style callbacks
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/acpi_piix4.c | 91 ++++++++++++++++++++++++------------------------------- 1 file changed, 40 insertions(+), 51 deletions(-)
hw/acpi_piix4.c +40 -51
@@ -531,68 +531,57 @@ static const MemoryRegionOps piix4_gpe_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; -static uint32_t pci_up_read (void *opaque,uint32_t addr) +static uint64_t pci_read (void *opaque,hwaddr addr,unsigned int size) {PIIX4PMState *s = opaque; -uint32_t val; - -/* Manufacture an "up" value to cause a device check on any hotplug -* slot with a device. Extra device checks are harmless. */ - val = s->pci0_slot_device_present & s->pci0_hotplug_enable; - -PIIX4_DPRINTF ("pci_up_read %x\n" , val); -return val; -} - -static uint32_t pci_down_read (void *opaque,uint32_t addr) -{ -PIIX4PMState *s = opaque; -uint32_t val = s->pci0_status.down; +uint32_t val =0 ; + +switch (addr) { +case PCI_UP_BASE -PCI_HOTPLUG_ADDR : +/* Manufacture an "up" value to cause a device check on any hotplug +* slot with a device. Extra device checks are harmless. */ + val = s->pci0_slot_device_present & s->pci0_hotplug_enable; +PIIX4_DPRINTF ("pci_up_read %x\n" , val); +break ; +case PCI_DOWN_BASE -PCI_HOTPLUG_ADDR : + val = s->pci0_status.down; +PIIX4_DPRINTF ("pci_down_read %x\n" , val); +break ; +case PCI_EJ_BASE -PCI_HOTPLUG_ADDR : +/* No feature defined yet */ +PIIX4_DPRINTF ("pci_features_read %x\n" , val); +break ; +case PCI_RMV_BASE -PCI_HOTPLUG_ADDR : + val = s->pci0_hotplug_enable; +break ; +default : +break ; + } -PIIX4_DPRINTF ("pci_down_read %x\n" , val);return val; } -static uint32_t pci_features_read (void *opaque,uint32_t addr) -{ -/* No feature defined yet */ -PIIX4_DPRINTF ("pci_features_read %x\n" ,0 ); -return 0 ; -} - -static void pciej_write (void *opaque,uint32_t addr,uint32_t val) -{ -acpi_piix_eject_slot (opaque, val); - -PIIX4_DPRINTF ("pciej write %x <== %d\n" , addr, val); -} - -static uint32_t pcirmv_read (void *opaque,uint32_t addr) +static void pci_write (void *opaque,hwaddr addr,uint64_t data, +unsigned int size) { -PIIX4PMState *s = opaque; - -return s->pci0_hotplug_enable; +switch (addr) { +case PCI_EJ_BASE -PCI_HOTPLUG_ADDR : +acpi_piix_eject_slot (opaque, (uint32_t )data); +PIIX4_DPRINTF ("pciej write %x <== % " PRIu64"\n" , addr, data); +break ; +default : +break ; + } }static const MemoryRegionOps piix4_pci_ops = { - .old_portio = (MemoryRegionPortio []) { - { - .offset =PCI_UP_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_up_read, - },{ - .offset =PCI_DOWN_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_down_read, - },{ - .offset =PCI_EJ_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_features_read, - .write = pciej_write, - },{ - .offset =PCI_RMV_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pcirmv_read, - }, -PORTIO_END_OF_LIST () + .read = pci_read, + .write = pci_write, + .endianness =DEVICE_NATIVE_ENDIAN , + .valid = { + .min_access_size =4 , + .max_access_size =4 , }, - .endianness =DEVICE_LITTLE_ENDIAN , }; static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
Signature · 1 line
1.7.10.4
Andreas Färber
AppliedRe: [Qemu-devel] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 22 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/acpi_piix4.c | 91 ++++++++++++++++++++++++------------------------------- 1 file changed, 40 insertions(+), 51 deletions(-)hw/acpi_piix4.c +1
-6@@ -531,68 +531,57 @@ static const MemoryRegionOps piix4_gpe_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; -static uint32_t pci_up_read (void *opaque,uint32_t addr) +static uint64_t pci_read (void *opaque,hwaddr addr,unsigned int size) {PIIX4PMState *s = opaque; -uint32_t val; - -/* Manufacture an "up" value to cause a device check on any hotplug -* slot with a device. Extra device checks are harmless. */ - val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
hw/acpi_piix4.c static const MemoryRegionOps piix4_gpe_ops = { · line 537
Quoted 10 lines
hw/acpi_piix4.c +1
-9- -PIIX4_DPRINTF ("pci_up_read %x\n" , val); -return val; -} - -static uint32_t pci_down_read (void *opaque,uint32_t addr) -{ -PIIX4PMState *s = opaque; -uint32_t val = s->pci0_status.down; +uint32_t val =0 ;
Here you are still using a local uint32_t variable...
Quoted 42 lines
hw/acpi_piix4.c +22
-16+ +switch (addr) { +case PCI_UP_BASE -PCI_HOTPLUG_ADDR : +/* Manufacture an "up" value to cause a device check on any hotplug +* slot with a device. Extra device checks are harmless. */ + val = s->pci0_slot_device_present & s->pci0_hotplug_enable; +PIIX4_DPRINTF ("pci_up_read %x\n" , val); +break ; +case PCI_DOWN_BASE -PCI_HOTPLUG_ADDR : + val = s->pci0_status.down; +PIIX4_DPRINTF ("pci_down_read %x\n" , val); +break ; +case PCI_EJ_BASE -PCI_HOTPLUG_ADDR : +/* No feature defined yet */ +PIIX4_DPRINTF ("pci_features_read %x\n" , val); +break ; +case PCI_RMV_BASE -PCI_HOTPLUG_ADDR : + val = s->pci0_hotplug_enable; +break ; +default : +break ; + } -PIIX4_DPRINTF ("pci_down_read %x\n" , val);return val; } -static uint32_t pci_features_read (void *opaque,uint32_t addr) -{ -/* No feature defined yet */ -PIIX4_DPRINTF ("pci_features_read %x\n" ,0 ); -return 0 ; -} - -static void pciej_write (void *opaque,uint32_t addr,uint32_t val) -{ -acpi_piix_eject_slot (opaque, val); - -PIIX4_DPRINTF ("pciej write %x <== %d\n" , addr, val); -} - -static uint32_t pcirmv_read (void *opaque,uint32_t addr)
hw/acpi_piix4.c static const MemoryRegionOps piix4_gpe_ops = { · line 570
Quoted 10 lines
hw/acpi_piix4.c +6
-3+static void pci_write (void *opaque,hwaddr addr,uint64_t data, +unsigned int size) { -PIIX4PMState *s = opaque; - -return s->pci0_hotplug_enable; +switch (addr) { +case PCI_EJ_BASE -PCI_HOTPLUG_ADDR : +acpi_piix_eject_slot (opaque, (uint32_t )data); +PIIX4_DPRINTF ("pciej write %x <== % " PRIu64"\n" , addr, data);
...but here you are printing hwaddr using %x. Fixing it up as follows:
hw/acpi_piix4.c +0 -0
@@ -567,7 +567,8 @@ static void pci_write(void *opaque, hwaddr addr,
uint64_t data,
switch (addr) {
case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
acpi_piix_eject_slot(opaque, (uint32_t)data);
- PIIX4_DPRINTF("pciej write %x <== % " PRIu64 "\n", addr, data);
+ PIIX4_DPRINTF("pciej write %" HWADDR_PRIx " <== % " PRIu64 "\n",
+ addr, data);
break;
default:
break;Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioport
Andreas
Quoted 34 lines
hw/acpi_piix4.c +10
-17+break ; +default : +break ; + } }static const MemoryRegionOps piix4_pci_ops = { - .old_portio = (MemoryRegionPortio []) { - { - .offset =PCI_UP_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_up_read, - },{ - .offset =PCI_DOWN_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_down_read, - },{ - .offset =PCI_EJ_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_features_read, - .write = pciej_write, - },{ - .offset =PCI_RMV_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pcirmv_read, - }, -PORTIO_END_OF_LIST () + .read = pci_read, + .write = pci_write, + .endianness =DEVICE_NATIVE_ENDIAN , + .valid = { + .min_access_size =4 , + .max_access_size =4 , }, - .endianness =DEVICE_LITTLE_ENDIAN , }; static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks
In reply to Hervé PoussineauAm 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 15 lines
static const MemoryRegionOps
piix4_pci_ops= {hw/acpi_piix4.c +0
-14- .old_portio = (MemoryRegionPortio[]) { - { - .offset =PCI_UP_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_up_read, - },{ - .offset =PCI_DOWN_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_down_read, - },{ - .offset =PCI_EJ_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_features_read, - .write = pciej_write, - },{ - .offset =PCI_RMV_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pcirmv_read,
hw/acpi_piix4.c static const MemoryRegionOps piix4_gpe_ops = { · line 595
Quoted 10 lines
hw/acpi_piix4.c +6
-3- }, -PORTIO_END_OF_LIST () + .read = pci_read, + .write = pci_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4, }, - .endianness = DEVICE_LITTLE_ENDIAN,
Ugh, question: Why is this changing Little Endian to native?
Andreas
Quoted 3 lines
};
static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks
Quoted 27 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 15 lines
static const MemoryRegionOps
piix4_pci_ops= {hw/acpi_piix4.c +0
-14- .old_portio = (MemoryRegionPortio[]) { - { - .offset =PCI_UP_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_up_read, - },{ - .offset =PCI_DOWN_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_down_read, - },{ - .offset =PCI_EJ_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pci_features_read, - .write = pciej_write, - },{ - .offset =PCI_RMV_BASE -PCI_HOTPLUG_ADDR , .len =4 , .size =4 , - .read = pcirmv_read,hw/acpi_piix4.c static const MemoryRegionOps piix4_gpe_ops = { · line 595
Quoted 10 lines
hw/acpi_piix4.c +6
-3- }, -PORTIO_END_OF_LIST () + .read = pci_read, + .write = pci_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4, }, - .endianness = DEVICE_LITTLE_ENDIAN,
Quoted 1 line
Ugh, question: Why is this changing Little Endian to native?
An error.
However, I was unable to find in the spec if this field is native on little endian. Not that it changes anything for now, as x86 is little endian... I will respin patches, except those already in your memory-ioport branch.
Hervé
PATCH 7/10 vga/qxl: do not use portio_list_init/portio_list_add
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/qxl.c | 4 +--- hw/vga.c | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-)
hw/qxl.c +1 -3
@@ -2048,7 +2048,6 @@ static int qxl_init_primary(PCIDevice *dev) {PCIQXLDevice *qxl =DO_UPCAST (PCIQXLDevice, pci, dev);VGACommonState *vga = &qxl->vga; -PortioList *qxl_vga_port_list =g_new (PortioList,1 );int rc; qxl->id =0 ;
@@ -2056,8 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev) vga->vram_size_mb = qxl->vga.vram_size >>20 ;vga_common_init (vga);vga_init (vga,pci_address_space (dev),pci_address_space_io (dev), false); -portio_list_init (qxl_vga_port_list, qxl_vga_portio_list, vga,"vga" ); -portio_list_add (qxl_vga_port_list,pci_address_space_io (dev),0x3b0 ); +isa_register_portio_list (NULL ,0x3b0 , qxl_vga_portio_list, vga,"vga" ); vga->ds =graphic_console_init (qxl_hw_update, qxl_hw_invalidate, qxl_hw_screen_dump, qxl_hw_text_update, qxl);
hw/vga.c +2 -6
@@ -2349,8 +2349,6 @@ void vga_init(VGACommonState *s, MemoryRegion *address_space, {MemoryRegion *vga_io_memory;const MemoryRegionPortio *vga_ports, *vbe_ports; -PortioList *vga_port_list =g_new (PortioList,1 ); -PortioList *vbe_port_list =g_new (PortioList,1 );qemu_register_reset (vga_reset, s);
@@ -2365,12 +2363,10 @@ void vga_init(VGACommonState *s, MemoryRegion *address_space, 1);memory_region_set_coalescing (vga_io_memory);if (init_vga_ports) { -portio_list_init (vga_port_list, vga_ports, s,"vga" ); -portio_list_add (vga_port_list, address_space_io,0x3b0 ); +isa_register_portio_list (NULL ,0x3b0 , vga_ports, s,"vga" ); }if (vbe_ports) { -portio_list_init (vbe_port_list, vbe_ports, s,"vbe" ); -portio_list_add (vbe_port_list, address_space_io,0x1ce ); +isa_register_portio_list (NULL ,0x1ce , vbe_ports, s,"vbe" ); } }
Signature · 1 line
1.7.10.4
PATCH 8/10 isa: use memory regions instead of portio_list_* functions
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- hw/isa-bus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- hw/isa.h | 2 +- 2 files changed, 125 insertions(+), 4 deletions(-)
hw/isa-bus.c +124 -3
@@ -104,19 +104,140 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)isa_init_ioport (dev, start); } +typedef struct PortioState { +const char *name;/* debug purposes */ +uint16_t start; +uint16_t offset; +const MemoryRegionPortio *pio_start; +void *opaque; +}PortioState ; + +static const MemoryRegionPortio *portio_find (const MemoryRegionPortio *mrp, +uint64_t offset, +unsigned int width,bool write, +bool smaller) +{ +for (; mrp->size; ++mrp) { +if (offset >= mrp->offset && offset < mrp->offset + mrp->len + && (width == mrp->size || (smaller && width < mrp->size)) + && (write ? (bool )mrp->write : (bool )mrp->read)) { +return mrp; + } + } +return NULL ; +} + +static uint64_t portio_read (void *opaque,hwaddr addr,unsigned int size) +{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, false, false); +if (mrp) { +return mrp->read (s->opaque, s->start + addr); + }else if (size ==2 ) { +uint64_t data; + mrp =portio_find (s->pio_start, addr,1 , false, false); +assert (mrp); + data = mrp->read (s->opaque, s->start + addr) | + (mrp->read (s->opaque, s->start + addr +1 ) <<8 ); +return data; + } +qemu_log_mask (LOG_GUEST_ERROR ,"%s: invalid read from 0x%x size=%d" , + s->name, s->start + (int )addr, size); +return -1U ; +} + +static void portio_write (void *opaque,hwaddr addr,uint64_t data, +unsigned int size) +{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, true, false); +if (mrp) { + mrp->write (s->opaque, s->start + addr, data); +return ; + }else if (size ==2 ) { + mrp =portio_find (s->pio_start, addr,1 , true, false); +assert (mrp); + mrp->write (s->opaque, s->start + addr, data &0xff ); + mrp->write (s->opaque, s->start + addr +1 , data >>8 ); +return ; + } +qemu_log_mask (LOG_GUEST_ERROR ,"%s: invalid write to 0x%x size=%d" , + s->name, s->start + (int )addr, size); +} + +static bool portio_accepts (void *opaque,hwaddr addr,unsigned int size, +bool is_write) +{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, is_write, true); +return (mrp !=NULL ); +} + +const MemoryRegionOps portio_ops = { + .read = portio_read, + .write = portio_write, + .valid.accepts = portio_accepts, + .endianness =DEVICE_LITTLE_ENDIAN , +}; + +static void isa_register_portio_list_1 (ISADevice *dev,uint16_t start, +uint16_t offset,uint16_t end, +const MemoryRegionPortio *pio_start, +void *opaque,const char *name) +{ +MemoryRegion *mr =g_new (MemoryRegion,1 ); +PortioState *s =g_new (PortioState,1 ); + + s->name = name; + s->start = start; + s->offset = offset; + s->pio_start = pio_start; + s->opaque = opaque; +memory_region_init_io (mr, &portio_ops, s, name, end - offset); +memory_region_add_subregion (isa_address_space_io (dev), + start + offset, mr); +} +void isa_register_portio_list (ISADevice *dev,uint16_t start,const MemoryRegionPortio *pio_start,void *opaque,const char *name) { -PortioList *piolist =g_new (PortioList,1 ); +const MemoryRegionPortio *pio, *first; +uint16_t end;/* START is how we should treat DEV, regardless of the actual contents of the portio array. This is how the old code actually handled e.g. the FDC device. */ isa_init_ioport (dev, start); -portio_list_init (piolist, pio_start, opaque, name); -portio_list_add (piolist, isabus->address_space_io, start); +assert (pio_start->size); + + first = pio_start; + end =0 ; +for (pio = pio_start; pio->size; pio++) { +assert (pio->offset >= first->offset); +if (pio->offset > first->offset + first->len) { +isa_register_portio_list_1 (dev, start, first->offset, end, + pio_start, opaque, name); + first = pio; + end =0 ; + } +if (pio->offset + pio->len > end) { + end = pio->offset + pio->len; + } + } + +isa_register_portio_list_1 (dev, start, first->offset, end, + pio_start, opaque, name); }static int isa_qdev_init (DeviceState *qdev)
hw/isa.h +1 -1
@@ -73,7 +73,7 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);* @dev: the ISADevice against which these are registered; may be NULL. * @start: the base I/O port against which the portio->offset is applied. * @portio: the ports, sorted by offset. -* @opaque: passed into the old_portio callbacks. +* @opaque: passed into the portio callbacks. * @name: passed into memory_region_init_io. */ void isa_register_portio_list(ISADevice *dev, uint16_t start,
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH 08/10] isa: use memory regions instead of portio_list_* functions
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 46 lines
Signed-off-by Hervé Poussineau <[email protected]>--- hw/isa-bus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- hw/isa.h | 2 +- 2 files changed, 125 insertions(+), 4 deletions(-)hw/isa-bus.c +32
-0@@ -104,19 +104,140 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)isa_init_ioport (dev, start); } +typedef struct PortioState { +const char *name;/* debug purposes */ +uint16_t start; +uint16_t offset; +const MemoryRegionPortio *pio_start; +void *opaque; +}PortioState ; + +static const MemoryRegionPortio *portio_find (const MemoryRegionPortio *mrp, +uint64_t offset, +unsigned int width,bool write, +bool smaller) +{ +for (; mrp->size; ++mrp) { +if (offset >= mrp->offset && offset < mrp->offset + mrp->len + && (width == mrp->size || (smaller && width < mrp->size)) + && (write ? (bool )mrp->write : (bool )mrp->read)) { +return mrp; + } + } +return NULL ; +} + +static uint64_t portio_read (void *opaque,hwaddr addr,unsigned int size) +{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, false, false); +if (mrp) { +return mrp->read (s->opaque, s->start + addr);
hw/isa-bus.c void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start) · line 148
Quoted 10 lines
hw/isa-bus.c +10
-0+ }else if (size ==2 ) { +uint64_t data; + mrp =portio_find (s->pio_start, addr,1 , false, false); +assert (mrp); + data = mrp->read (s->opaque, s->start + addr) | + (mrp->read (s->opaque, s->start + addr +1 ) <<8 ); +return data; + } +qemu_log_mask (LOG_GUEST_ERROR ,"%s: invalid read from 0x%x size=%d" , + s->name, s->start + (int )addr, size);
I note that patch 10/10 shows that memory.c does it similarly, assuming Little Endian for size == 2 and ignoring assembled size == 4.
Quoted 13 lines
hw/isa-bus.c +13
-0+return -1U ; +} + +static void portio_write (void *opaque,hwaddr addr,uint64_t data, +unsigned int size) +{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, true, false); +if (mrp) { + mrp->write (s->opaque, s->start + addr, data);
hw/isa-bus.c void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start) · line 171
Quoted 10 lines
hw/isa-bus.c +10
-0+return ; + }else if (size ==2 ) { + mrp =portio_find (s->pio_start, addr,1 , true, false); +assert (mrp); + mrp->write (s->opaque, s->start + addr, data &0xff ); + mrp->write (s->opaque, s->start + addr +1 , data >>8 ); +return ; + } +qemu_log_mask (LOG_GUEST_ERROR ,"%s: invalid write to 0x%x size=%d" , + s->name, s->start + (int )addr, size);
Ditto.
Quoted 4 lines
hw/isa-bus.c +4
-0+} + +static bool portio_accepts (void *opaque,hwaddr addr,unsigned int size, +bool is_write)
hw/isa-bus.c void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start) · line 185
Quoted 10 lines
hw/isa-bus.c +10
-0+{ +const PortioState *s = opaque; +const MemoryRegionPortio *mrp; + + addr += s->offset; + mrp =portio_find (s->pio_start, addr, size, is_write, true); +return (mrp !=NULL ); +} + +const MemoryRegionOps portio_ops = {
static const?
Quoted 66 lines
hw/isa-bus.c +45
-3+ .read = portio_read, + .write = portio_write, + .valid.accepts = portio_accepts, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void isa_register_portio_list_1 (ISADevice *dev,uint16_t start, +uint16_t offset,uint16_t end, +const MemoryRegionPortio *pio_start, +void *opaque,const char *name) +{ +MemoryRegion *mr =g_new (MemoryRegion,1 ); +PortioState *s =g_new (PortioState,1 ); + + s->name = name; + s->start = start; + s->offset = offset; + s->pio_start = pio_start; + s->opaque = opaque; +memory_region_init_io (mr, &portio_ops, s, name, end - offset); +memory_region_add_subregion (isa_address_space_io (dev), + start + offset, mr); +} +void isa_register_portio_list (ISADevice *dev,uint16_t start,const MemoryRegionPortio *pio_start,void *opaque,const char *name) { -PortioList *piolist =g_new (PortioList,1 ); +const MemoryRegionPortio *pio, *first; +uint16_t end;/* START is how we should treat DEV, regardless of the actual contents of the portio array. This is how the old code actually handled e.g. the FDC device. */ isa_init_ioport (dev, start); -portio_list_init (piolist, pio_start, opaque, name); -portio_list_add (piolist, isabus->address_space_io, start); +assert (pio_start->size); + + first = pio_start; + end =0 ; +for (pio = pio_start; pio->size; pio++) { +assert (pio->offset >= first->offset); +if (pio->offset > first->offset + first->len) { +isa_register_portio_list_1 (dev, start, first->offset, end, + pio_start, opaque, name); + first = pio; + end =0 ; + } +if (pio->offset + pio->len > end) { + end = pio->offset + pio->len; + } + } + +isa_register_portio_list_1 (dev, start, first->offset, end, + pio_start, opaque, name); }static int isa_qdev_init (DeviceState *qdev)hw/isa.h +0
-0
hw/isa.h void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start); · line 79
Quoted 8 lines
hw/isa.h +1
-1@@ -73,7 +73,7 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);* @dev: the ISADevice against which these are registered; may be NULL. * @start: the base I/O port against which the portio->offset is applied. * @portio: the ports, sorted by offset. -* @opaque: passed into the old_portio callbacks. +* @opaque: passed into the portio callbacks. * @name: passed into memory_region_init_io. */ void isa_register_portio_list(ISADevice *dev, uint16_t start,
Otherwise looks okay to me, but I'd appreciate another pair of eyes.
AFAIU this moves the current implementation from ioport.c (09/10) and memory.c (10/10) into isa-bus.c, to remove MemoryRegionOps::old_portio in 10/10. But ISA keeps using MemoryRegionPortio unlike memory.c itself.
I wonder if it were not a cleaner course of action to remove isa_register_portio_list() completely by converting callers to use isa_register_ioport() with a MemoryRegion owned by the caller? Remaining MemoryRegionPortio lists after this series below.
Regards,
Andreas
dma.c:
/* 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(),
};fdc.c:
static const MemoryRegionPortio fdc_portio_list[] = {
{ 1, 5, 1, .read = fdctrl_read, .write = fdctrl_write },
{ 7, 1, 1, .read = fdctrl_read, .write = fdctrl_write },
PORTIO_END_OF_LIST(),
};gus.c:
static const MemoryRegionPortio gus_portio_list1[] = {
{0x000, 1, 1, .write = gus_writeb },
{0x000, 1, 2, .write = gus_writew },
{0x006, 10, 1, .read = gus_readb, .write = gus_writeb },
{0x006, 10, 2, .read = gus_readw, .write = gus_writew },
{0x100, 8, 1, .read = gus_readb, .write = gus_writeb },
{0x100, 8, 2, .read = gus_readw, .write = gus_writew },
PORTIO_END_OF_LIST (),
};static const MemoryRegionPortio gus_portio_list2[] = {
{0, 1, 1, .read = gus_readb },
{0, 1, 2, .read = gus_readw },
PORTIO_END_OF_LIST (),
};ide/core.c:
static const MemoryRegionPortio ide_portio_list[] = {
{ 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write },
{ 0, 2, 2, .read = ide_data_readw, .write = ide_data_writew },
{ 0, 4, 4, .read = ide_data_readl, .write = ide_data_writel },
PORTIO_END_OF_LIST(),
};static const MemoryRegionPortio ide_portio2_list[] = {
{ 0, 1, 1, .read = ide_status_read, .write = ide_cmd_write },
PORTIO_END_OF_LIST(),
};parallel.c:
static const MemoryRegionPortio isa_parallel_portio_hw_list[] = {
{ 0, 8, 1,
.read = parallel_ioport_read_hw,
.write = parallel_ioport_write_hw },
{ 4, 1, 2,
.read = parallel_ioport_eppdata_read_hw2,
.write = parallel_ioport_eppdata_write_hw2 },
{ 4, 1, 4,
.read = parallel_ioport_eppdata_read_hw4,
.write = parallel_ioport_eppdata_write_hw4 },
{ 0x400, 8, 1,
.read = parallel_ioport_ecp_read,
.write = parallel_ioport_ecp_write },
PORTIO_END_OF_LIST(),
};
static const MemoryRegionPortio isa_parallel_portio_sw_list[] = {
{ 0, 8, 1,
.read = parallel_ioport_read_sw,
.write = parallel_ioport_write_sw },
PORTIO_END_OF_LIST(),
};
qxl.c:
static const MemoryRegionPortio qxl_vga_portio_list[] = {
{ 0x04, 2, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3b4 */
{ 0x0a, 1, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3ba */
{ 0x10, 16, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3c0 */
{ 0x24, 2, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3d4 */
{ 0x2a, 1, 1, .read = vga_ioport_read,
.write = qxl_vga_ioport_write }, /* 3da */
PORTIO_END_OF_LIST(),
};sb16.c:
static const MemoryRegionPortio sb16_ioport_list[] = {
{ 4, 1, 1, .write = mixer_write_indexb },
{ 4, 1, 2, .write = mixer_write_indexw },
{ 5, 1, 1, .read = mixer_read, .write = mixer_write_datab },
{ 6, 1, 1, .read = dsp_read, .write = dsp_write },
{ 10, 1, 1, .read = dsp_read },
{ 12, 1, 1, .write = dsp_write },
{ 12, 4, 1, .read = dsp_read },
PORTIO_END_OF_LIST (),
};vga.c:
static const MemoryRegionPortio vga_portio_list[] = {
{ 0x04, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3b4 */
{ 0x0a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3ba */
{ 0x10, 16, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3c0 */
{ 0x24, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3d4 */
{ 0x2a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3da */
PORTIO_END_OF_LIST(),
};static const MemoryRegionPortio vbe_portio_list[] = {
{ 0, 1, 2, .read = vbe_ioport_read_index, .write =
vbe_ioport_write_index },
# ifdef TARGET_I386
{ 1, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
# endif
{ 2, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
PORTIO_END_OF_LIST(),
};Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
PATCH 9/10 ioport: remove now useless portio_list_* functions
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- include/exec/ioport.h | 19 -------- ioport.c | 121 ------------------------------------------------- 2 files changed, 140 deletions(-)
include/exec/ioport.h +0 -19
@@ -56,23 +56,4 @@ uint32_t cpu_inl(pio_addr_t addr);struct MemoryRegion ;struct MemoryRegionPortio ; -typedef struct PortioList { -const struct MemoryRegionPortio *ports; -struct MemoryRegion *address_space; -unsigned nr; -struct MemoryRegion **regions; -struct MemoryRegion **aliases; -void *opaque; -const char *name; -}PortioList ; - -void portio_list_init (PortioList *piolist, -const struct MemoryRegionPortio *callbacks, -void *opaque,const char *name); -void portio_list_destroy (PortioList *piolist); -void portio_list_add (PortioList *piolist, -struct MemoryRegion *address_space, -uint32_t addr); -void portio_list_del (PortioList *piolist); -#endif /* IOPORT_H */
ioport.c +0 -121
124 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 10/10 memory: remove old_portio-style callbacks support
In reply to Hervé PoussineauSigned-off-by Hervé Poussineau <[email protected]>
--- include/exec/memory.h | 4 ---- memory.c | 44 -------------------------------------------- 2 files changed, 48 deletions(-)
include/exec/memory.h +0 -4
@@ -103,10 +103,6 @@ struct MemoryRegionOps {bool unaligned; } impl; -/* If .read and .write are not present, old_portio may be used for -* backwards compatibility with old portio registration -*/ -const MemoryRegionPortio *old_portio;/* If .read and .write are not present, old_mmio may be used for * backwards compatibility with old mmio registration */
memory.c +0 -44
62 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH 10/10] memory: remove old_portio-style callbacks support
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 11 lines
Signed-off-by Hervé Poussineau <[email protected]>--- include/exec/memory.h | 4 ---- memory.c | 44 -------------------------------------------- 2 files changed, 48 deletions(-)include/exec/memory.h +0
-0
include/exec/memory.h struct MemoryRegionOps { · line 108
Quoted 10 lines
include/exec/memory.h +0
-4@@ -103,10 +103,6 @@ struct MemoryRegionOps {bool unaligned; } impl; -/* If .read and .write are not present, old_portio may be used for -* backwards compatibility with old portio registration -*/ -const MemoryRegionPortio *old_portio;/* If .read and .write are not present, old_mmio may be used for * backwards compatibility with old mmio registration */
Can't struct MemoryRegionPortio be removed as part of the patch?
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 10/10] memory: remove old_portio-style callbacks support
Quoted 15 lines · Andreas Färber
Am 04.01.2013 22:29, schrieb Hervé Poussineau:
Quoted 14 lines
Signed-off-by Hervé Poussineau <[email protected]>--- include/exec/memory.h | 4 ---- memory.c | 44 -------------------------------------------- 2 files changed, 48 deletions(-)include/exec/memory.h +0
-0@@ -103,10 +103,6 @@ struct MemoryRegionOps {bool unaligned; } impl;
Quoted 9 lines
include/exec/memory.h struct MemoryRegionOps { · line 108
Quoted 7 lines
include/exec/memory.h +0
-4-/* If .read and .write are not present, old_portio may be used for -* backwards compatibility with old portio registration -*/ -const MemoryRegionPortio *old_portio;/* If .read and .write are not present, old_mmio may be used for * backwards compatibility with old mmio registration */ Can't struct MemoryRegionPortio be removed as part of the patch?
No, it is also used as parameter for the isa_register_portio_list() function. isa_register_portio_list() parses this list and creates corresponding MemoryRegions.
Hervé