| Message ID | AANLkTil-VFu1P4q_4athK2r1XkBgZh47NGjj2wtZw3G9@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Blue Swirl wrote: > Hi all, > > I finally refreshed some of my monitor patches. PCI and HPET patches > (attached, they don't apply anymore) need more work because of the new > monitor design. > > The patches provide a method for devices to register new monitor > commands. This fixes some design problems, like useless > pic_info/irq_info functions for most architectures. > > I think automation via qdev field was requested the last time. I added > something like this, though it doesn't fit the cases where several > functions need to be registered. > > Comments? I'll soon send out a series that adds "device_show <qdev-path>" to visualize the full state, something that will at least obsolete "info pic" and "info hpet" sooner or later, maybe even more. Also, I would like to qdev'ify CPUs in order to make them reachable for device_show (which evaluates qdev->info.vmstate). I'm not sure: How many use cases for a "dev_info" would remain? Moreover, to dump statistics, we should rather add some "device_stats <qdev-path>" command instead of mixing both usages under an info command. Jan
On 5/13/10, Jan Kiszka <jan.kiszka@web.de> wrote: > Blue Swirl wrote: > > Hi all, > > > > I finally refreshed some of my monitor patches. PCI and HPET patches > > (attached, they don't apply anymore) need more work because of the new > > monitor design. > > > > The patches provide a method for devices to register new monitor > > commands. This fixes some design problems, like useless > > pic_info/irq_info functions for most architectures. > > > > I think automation via qdev field was requested the last time. I added > > something like this, though it doesn't fit the cases where several > > functions need to be registered. > > > > Comments? > > > I'll soon send out a series that adds "device_show <qdev-path>" to > visualize the full state, something that will at least obsolete "info > pic" and "info hpet" sooner or later, maybe even more. Also, I would > like to qdev'ify CPUs in order to make them reachable for device_show > (which evaluates qdev->info.vmstate). That sounds like much better design. But for example 'info pci' shows different things compared to 'info qdev'. And what about 'info usbhost', there is no qdev? > I'm not sure: How many use cases for a "dev_info" would remain? > Moreover, to dump statistics, we should rather add some "device_stats > <qdev-path>" command instead of mixing both usages under an info command. Not too many. I think the VMState structure (with no connection to savevm/loadvm/migration) would be useful to define and dump also statistics. But because most statistics need to be enabled at compile phase, they are probably not very widely used.
Blue Swirl wrote: > On 5/13/10, Jan Kiszka <jan.kiszka@web.de> wrote: >> Blue Swirl wrote: >> > Hi all, >> > >> > I finally refreshed some of my monitor patches. PCI and HPET patches >> > (attached, they don't apply anymore) need more work because of the new >> > monitor design. >> > >> > The patches provide a method for devices to register new monitor >> > commands. This fixes some design problems, like useless >> > pic_info/irq_info functions for most architectures. >> > >> > I think automation via qdev field was requested the last time. I added >> > something like this, though it doesn't fit the cases where several >> > functions need to be registered. >> > >> > Comments? >> >> >> I'll soon send out a series that adds "device_show <qdev-path>" to >> visualize the full state, something that will at least obsolete "info >> pic" and "info hpet" sooner or later, maybe even more. Also, I would >> like to qdev'ify CPUs in order to make them reachable for device_show >> (which evaluates qdev->info.vmstate). > > That sounds like much better design. But for example 'info pci' shows > different things compared to 'info qdev'. And what about 'info > usbhost', there is no qdev? That should be addressed differently (if there is a need to change it). My point is basically about things that are (or will be) qdev related - just as dev_info was suggesting. > >> I'm not sure: How many use cases for a "dev_info" would remain? >> Moreover, to dump statistics, we should rather add some "device_stats >> <qdev-path>" command instead of mixing both usages under an info command. > > Not too many. I think the VMState structure (with no connection to > savevm/loadvm/migration) would be useful to define and dump also > statistics. But because most statistics need to be enabled at compile > phase, they are probably not very widely used. If they are special, then I would vote for a separate stats infrastructure with its own data types where required. If certain stats should rather be enabled by default, then there is actually the question if they shouldn't be migrated as well, thus become part of the related VMState definitions. Just thoughts, I haven't done any investigations on the current situation. However, I'm a fan of a plug-in interface for the existing info monitor command. That would allow to register things dynamically that do not fit in any regular structure without requiring stubs or tons of #ifdefs. What about this as a first step? Jan
On 5/13/10, Jan Kiszka <jan.kiszka@web.de> wrote: > Blue Swirl wrote: > > On 5/13/10, Jan Kiszka <jan.kiszka@web.de> wrote: > >> Blue Swirl wrote: > >> > Hi all, > >> > > >> > I finally refreshed some of my monitor patches. PCI and HPET patches > >> > (attached, they don't apply anymore) need more work because of the new > >> > monitor design. > >> > > >> > The patches provide a method for devices to register new monitor > >> > commands. This fixes some design problems, like useless > >> > pic_info/irq_info functions for most architectures. > >> > > >> > I think automation via qdev field was requested the last time. I added > >> > something like this, though it doesn't fit the cases where several > >> > functions need to be registered. > >> > > >> > Comments? > >> > >> > >> I'll soon send out a series that adds "device_show <qdev-path>" to > >> visualize the full state, something that will at least obsolete "info > >> pic" and "info hpet" sooner or later, maybe even more. Also, I would > >> like to qdev'ify CPUs in order to make them reachable for device_show > >> (which evaluates qdev->info.vmstate). > > > > That sounds like much better design. But for example 'info pci' shows > > different things compared to 'info qdev'. And what about 'info > > usbhost', there is no qdev? > > > That should be addressed differently (if there is a need to change it). > My point is basically about things that are (or will be) qdev related - > just as dev_info was suggesting. > > > > > >> I'm not sure: How many use cases for a "dev_info" would remain? > >> Moreover, to dump statistics, we should rather add some "device_stats > >> <qdev-path>" command instead of mixing both usages under an info command. > > > > Not too many. I think the VMState structure (with no connection to > > savevm/loadvm/migration) would be useful to define and dump also > > statistics. But because most statistics need to be enabled at compile > > phase, they are probably not very widely used. > > > If they are special, then I would vote for a separate stats > infrastructure with its own data types where required. If certain stats > should rather be enabled by default, then there is actually the question > if they shouldn't be migrated as well, thus become part of the related > VMState definitions. Just thoughts, I haven't done any investigations on > the current situation. Currently for example PIC statistics (like i8259.c and slavio_intctl.c) are not saved or migrated. Statistics generation need a specially compiled QEMU, so I don't think statistics saving or migration will be any normal use case. > However, I'm a fan of a plug-in interface for the existing info monitor > command. That would allow to register things dynamically that do not fit > in any regular structure without requiring stubs or tons of #ifdefs. > What about this as a first step? I think qdev dump approach would be better for current and future qdev objects. But I'll try if my version makes non-qdev stuff ('info usbhost' or something) simpler. The main benefit with the registration is IMHO that the caller can specify a state variable when registering and this variable is also available at callback time. This allows many cleanups.
From 13952817d5e4f01333c3d35d05da916719ed2322 Mon Sep 17 00:00:00 2001 From: Blue Swirl <blauwirbel@gmail.com> Date: Wed, 9 Sep 2009 17:40:35 +0000 Subject: [PATCH] x86: use device info for hpet Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- hw/pc.c | 12 ++++++++++++ monitor.c | 10 ---------- qemu-monitor.hx | 2 -- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 6292001..39a0970 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -1108,6 +1108,17 @@ static CPUState *pc_new_cpu(const char *cpu_model) return env; } +static void info_hpet(Monitor *mon, void *opaque) +{ + monitor_printf(mon, "HPET is %s by QEMU\n", + (no_hpet) ? "disabled" : "enabled"); +} + +static const struct MonDevInfo hpet_info = { + .dev_name = "hpet", + .dev_info_cb = info_hpet, +}; + /* PC hardware initialisation */ static void pc_init1(ram_addr_t ram_size, const char *boot_device, @@ -1327,6 +1338,7 @@ static void pc_init1(ram_addr_t ram_size, if (!no_hpet) { hpet_init(isa_irq); } + monitor_register_device_info(&hpet_info, NULL); for(i = 0; i < MAX_SERIAL_PORTS; i++) { if (serial_hds[i]) { diff --git a/monitor.c b/monitor.c index 3d18885..202c457 100644 --- a/monitor.c +++ b/monitor.c @@ -315,14 +315,6 @@ static void do_info_name(Monitor *mon) monitor_printf(mon, "%s\n", qemu_name); } -#if defined(TARGET_I386) -static void do_info_hpet(Monitor *mon) -{ - monitor_printf(mon, "HPET is %s by QEMU\n", - (no_hpet) ? "disabled" : "enabled"); -} -#endif - static void do_info_uuid(Monitor *mon) { monitor_printf(mon, UUID_FMT "\n", qemu_uuid[0], qemu_uuid[1], @@ -1851,8 +1843,6 @@ static const mon_cmd_t info_cmds[] = { #if defined(TARGET_I386) { "mem", "", mem_info, "", "show the active virtual memory mappings", }, - { "hpet", "", do_info_hpet, - "", "show state of HPET", }, #endif { "jit", "", do_info_jit, "", "show dynamic compiler info", }, diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 54078e2..26f9cf7 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -49,8 +49,6 @@ show the command line history show virtual to physical memory mappings (i386 only) @item info mem show the active virtual memory mappings (i386 only) -@item info hpet -show state of HPET (i386 only) @item info kvm show KVM information @item info usb -- 1.5.6.5