qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 05/10] vmport: convert to qdev
Date: Tue, 15 Feb 2011 11:22:17 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux)

Blue Swirl <address@hidden> writes:

> On Sat, Feb 12, 2011 at 6:57 PM, Markus Armbruster <address@hidden> wrote:
>> 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.
>
> 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]