* [Qemu-devel] [PATCH, RFC 0/4] monitor device info infrastructure
@ 2010-05-12 20:56 Blue Swirl
2010-05-13 7:40 ` [Qemu-devel] " Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2010-05-12 20:56 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]
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?
Blue Swirl (4):
monitor: add device info infrastructure
qdev: automatically register device info
x86/Sparc32: use device info for pic and irq
PPC: use device info for CPU statistics
cpu-all.h | 3 --
hw/an5206.c | 9 ------
hw/arm_pic.c | 10 ------
hw/cris_pic_cpu.c | 5 ---
hw/i8259.c | 50 ++++++++++++++++++++-----------
hw/microblaze_pic_cpu.c | 5 ---
hw/pc.h | 2 -
hw/qdev.c | 3 ++
hw/qdev.h | 3 ++
hw/shix.c | 10 ------
hw/slavio_intctl.c | 26 ++++++++++------
hw/sun4c_intctl.c | 16 +++++++++-
hw/sun4m.c | 15 +---------
hw/sun4m.h | 8 -----
hw/sun4u.c | 8 -----
monitor.c | 74 ++++++++++++++++++++++++++---------------------
monitor.h | 10 ++++++
qemu-monitor.hx | 19 ++++++++----
target-ppc/cpu.h | 2 +
target-ppc/helper.c | 4 ++
target-ppc/translate.c | 45 +++++++++++++++++++---------
21 files changed, 169 insertions(+), 158 deletions(-)
[-- Attachment #2: 0001-PCI-use-device-info.patch --]
[-- Type: text/x-diff, Size: 4583 bytes --]
From ee7796646db16bd36be6d6d128e87acd0e70fd32 Mon Sep 17 00:00:00 2001
From: Blue Swirl <blauwirbel@gmail.com>
Date: Wed, 9 Sep 2009 17:40:35 +0000
Subject: [PATCH] PCI: use device info
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
hw/pci.c | 51 ++++++++++++++++++++++++++++-----------------------
hw/pci.h | 2 --
monitor.c | 2 --
qemu-monitor.hx | 2 --
4 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index c12b0be..225efbf 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -98,6 +98,13 @@ static void pci_bus_reset(void *opaque)
}
}
+static void pci_info(Monitor *mon, void *opaque);
+
+static const struct MonDevInfo mon_pci_info = {
+ .dev_name = "pci",
+ .dev_info_cb = pci_info,
+};
+
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int devfn_min, int nirq)
@@ -116,6 +123,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
first_bus = bus;
vmstate_register(nbus++, &vmstate_pcibus, bus);
qemu_register_reset(pci_bus_reset, bus);
+ monitor_register_device_info(&mon_pci_info, bus);
return bus;
}
@@ -693,9 +701,24 @@ static const pci_class_desc pci_class_descriptions[] =
{ 0, NULL}
};
-static void pci_info_device(PCIDevice *d)
+static void pci_bus_for_each_device(PCIBus *bus,
+ void (*fn)(PCIDevice *d, void *fn_opaque),
+ void *fn_opaque)
+{
+ PCIDevice *d;
+ int devfn;
+
+ for(devfn = 0; devfn < 256; devfn++) {
+ d = bus->devices[devfn];
+ if (d) {
+ fn(d, fn_opaque);
+ }
+ }
+}
+
+static void pci_info_device(PCIDevice *d, void *opaque)
{
- Monitor *mon = cur_mon;
+ Monitor *mon = opaque;
int i, class;
PCIIORegion *r;
const pci_class_desc *desc;
@@ -737,31 +760,13 @@ static void pci_info_device(PCIDevice *d)
}
}
monitor_printf(mon, " id \"%s\"\n", d->qdev.id ? d->qdev.id : "");
- if (class == 0x0604 && d->config[0x19] != 0) {
- pci_for_each_device(d->config[0x19], pci_info_device);
- }
}
-void pci_for_each_device(int bus_num, void (*fn)(PCIDevice *d))
+static void pci_info(Monitor *mon, void *opaque)
{
- PCIBus *bus = first_bus;
- PCIDevice *d;
- int devfn;
-
- while (bus && bus->bus_num != bus_num)
- bus = bus->next;
- if (bus) {
- for(devfn = 0; devfn < 256; devfn++) {
- d = bus->devices[devfn];
- if (d)
- fn(d);
- }
- }
-}
+ PCIBus *bus = opaque;
-void pci_info(Monitor *mon)
-{
- pci_for_each_device(0, pci_info_device);
+ pci_bus_for_each_device(bus, pci_info_device, mon);
}
PCIDevice *pci_create(const char *name, const char *devaddr)
diff --git a/hw/pci.h b/hw/pci.h
index 5340bbb..b732f00 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -250,14 +250,12 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len);
uint32_t pci_data_read(void *opaque, uint32_t addr, int len);
int pci_bus_num(PCIBus *s);
-void pci_for_each_device(int bus_num, void (*fn)(PCIDevice *d));
PCIBus *pci_find_bus(int bus_num);
PCIDevice *pci_find_device(int bus_num, int slot, int function);
int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
unsigned *slotp);
-void pci_info(Monitor *mon);
PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
pci_map_irq_fn map_irq, const char *name);
diff --git a/monitor.c b/monitor.c
index 2c763e8..3d18885 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1844,8 +1844,6 @@ static const mon_cmd_t info_cmds[] = {
"", "show infos for each CPU" },
{ "history", "", do_info_history,
"", "show the command line history", },
- { "pci", "", pci_info,
- "", "show PCI info", },
#if defined(TARGET_I386) || defined(TARGET_SH4)
{ "tlb", "", tlb_info,
"", "show virtual to physical memory mappings", },
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 0a9c591..54078e2 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -45,8 +45,6 @@ show the cpu registers
show infos for each CPU
@item info history
show the command line history
-@item info pci
-show emulated PCI device info
@item info tlb
show virtual to physical memory mappings (i386 only)
@item info mem
--
1.5.6.5
[-- Attachment #3: 0002-x86-use-device-info-for-hpet.patch --]
[-- Type: text/x-diff, Size: 2476 bytes --]
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] Re: [PATCH, RFC 0/4] monitor device info infrastructure
2010-05-12 20:56 [Qemu-devel] [PATCH, RFC 0/4] monitor device info infrastructure Blue Swirl
@ 2010-05-13 7:40 ` Jan Kiszka
2010-05-13 18:50 ` Blue Swirl
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2010-05-13 7:40 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] Re: [PATCH, RFC 0/4] monitor device info infrastructure
2010-05-13 7:40 ` [Qemu-devel] " Jan Kiszka
@ 2010-05-13 18:50 ` Blue Swirl
2010-05-13 19:38 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2010-05-13 18:50 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] Re: [PATCH, RFC 0/4] monitor device info infrastructure
2010-05-13 18:50 ` Blue Swirl
@ 2010-05-13 19:38 ` Jan Kiszka
2010-05-13 20:01 ` Blue Swirl
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2010-05-13 19:38 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] Re: [PATCH, RFC 0/4] monitor device info infrastructure
2010-05-13 19:38 ` Jan Kiszka
@ 2010-05-13 20:01 ` Blue Swirl
0 siblings, 0 replies; 5+ messages in thread
From: Blue Swirl @ 2010-05-13 20:01 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-13 20:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-12 20:56 [Qemu-devel] [PATCH, RFC 0/4] monitor device info infrastructure Blue Swirl
2010-05-13 7:40 ` [Qemu-devel] " Jan Kiszka
2010-05-13 18:50 ` Blue Swirl
2010-05-13 19:38 ` Jan Kiszka
2010-05-13 20:01 ` Blue Swirl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).