Blue Swirl <blauwir...@gmail.com> writes: > On Sat, Feb 12, 2011 at 6:57 PM, Markus Armbruster <arm...@redhat.com> wrote: >> Blue Swirl <blauwir...@gmail.com> writes: >> >>> Signed-off-by: Blue Swirl <blauwir...@gmail.com> >>> --- >>> hw/pc.h | 1 - >>> hw/pc_piix.c | 2 -- >>> hw/vmport.c | 24 +++++++++++++++++++++--- >>> 3 files changed, 21 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/pc.h b/hw/pc.h >>> index a048768..603a2a3 100644 >>> --- a/hw/pc.h >>> +++ b/hw/pc.h >>> @@ -65,7 +65,6 @@ void hpet_pit_disable(void); >>> void hpet_pit_enable(void); >>> >>> /* vmport.c */ >>> -void vmport_init(void); >>> void vmport_register(unsigned char command, IOPortReadFunc *func, >>> void *opaque); >>> >>> /* vmmouse.c */ >>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >>> index 7b74473..d0bd0cd 100644 >>> --- a/hw/pc_piix.c >>> +++ b/hw/pc_piix.c >>> @@ -86,8 +86,6 @@ static void pc_init1(ram_addr_t ram_size, >>> >>> pc_cpus_init(cpu_model); >>> >>> - vmport_init(); >>> - >>> /* allocate ram and load rom/bios */ >>> pc_memory_init(ram_size, kernel_filename, kernel_cmdline, >>> initrd_filename, >>> &below_4g_mem_size, &above_4g_mem_size); >>> diff --git a/hw/vmport.c b/hw/vmport.c >>> index 6c9d7c9..4432be0 100644 >>> --- a/hw/vmport.c >>> +++ b/hw/vmport.c >>> @@ -26,6 +26,7 @@ >>> #include "pc.h" >>> #include "sysemu.h" >>> #include "kvm.h" >>> +#include "qdev.h" >>> >>> //#define VMPORT_DEBUG >>> >>> @@ -37,6 +38,7 @@ >>> >>> typedef struct _VMPortState >>> { >>> + ISADevice dev; >>> IOPortReadFunc *func[VMPORT_ENTRIES]; >>> void *opaque[VMPORT_ENTRIES]; >>> } VMPortState; >>> @@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void >>> *opaque, uint32_t addr) >>> return ram_size; >>> } >>> >>> -void vmport_init(void) >>> +static int vmport_initfn(ISADevice *dev) >>> { >>> - register_ioport_read(0x5658, 1, 4, vmport_ioport_read, &port_state); >>> - register_ioport_write(0x5658, 1, 4, vmport_ioport_write, &port_state); >>> + VMPortState *s = DO_UPCAST(VMPortState, dev, dev); >>> >>> + register_ioport_read(0x5658, 1, 4, vmport_ioport_read, &s); >>> + register_ioport_write(0x5658, 1, 4, vmport_ioport_write, &s); >>> + isa_init_ioport(dev, 0x5658); >>> /* Register some generic port commands */ >>> vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL); >>> vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL); >>> + return 0; >>> } >>> + >>> +static ISADeviceInfo vmport_info = { >>> + .qdev.name = "vmport", >>> + .qdev.size = sizeof(VMPortState), >>> + .qdev.no_user = 1, >>> + .init = vmport_initfn, >>> +}; >>> + >>> +static void vmport_dev_register(void) >>> +{ >>> + isa_qdev_register(&vmport_info); >>> +} >>> +device_init(vmport_dev_register) >> >> Old code has pc_init1() call vmport_init(). Where does your code create >> qdev "vmport"? And what's happening with port_state? It's still used >> by vmport_register(), but no longer connected to the I/O ports. Can't >> see how vmport_register() has any effect anymore. > > I fixed it in the committed version.
Did you post v2 to the list for review? >> Have you tested this? > > Sure. Here's how your v2 creates and initializes the qdev: diff --git a/hw/pc.c b/hw/pc.c index fcee09a..c698161 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -1133,6 +1133,7 @@ void pc_basic_device_init(qemu_irq *isa_irq, a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2); i8042 = isa_create_simple("i8042"); i8042_setup_a20_line(i8042, &a20_line[0]); + vmport_init(); vmmouse_init(i8042); port92 = isa_create_simple("port92"); port92_init(port92, &a20_line[1]); This allocates a new VMPortState, and registers callbacks for port 5658: @@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t addr) return ram_size; } -void vmport_init(void) +static int vmport_initfn(ISADevice *dev) { - register_ioport_read(0x5658, 1, 4, vmport_ioport_read, &port_state); - register_ioport_write(0x5658, 1, 4, vmport_ioport_write, &port_state); + VMPortState *s = DO_UPCAST(VMPortState, dev, dev); + register_ioport_read(0x5658, 1, 4, vmport_ioport_read, &s); + register_ioport_write(0x5658, 1, 4, vmport_ioport_write, &s); + isa_init_ioport(dev, 0x5658); /* Register some generic port commands */ vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL); vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL); + return 0; } The callbacks receive the qdev VMPortState as argument. vmport_register() still updates global port_state. I.e. it no longer has any effect whatsoever on what the ports do. Maybe I'm dense, but I can't see how this can work.