[Top][All Lists]
[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.