Re: [Qemu-devel] [PATCH 05/10] vmport: convert to qdev

[Markus Armbruster]

Blue Swirl <address@hidden> writes:

> Signed-off-by: Blue Swirl <address@hidden>
> ---
>  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.

Have you tested this?