On 12/5/24 05:00, Julian Ganz wrote:
Hi Pierrick,
December 5, 2024 at 12:14 AM, "Pierrick Bouvier" wrote:
On 12/2/24 11:26, Julian Ganz wrote:
+typedef struct {
+ uint64_t interrupts;
+ uint64_t exceptions;
+ uint64_t hostcalls;
+ bool active;
The active field can be removed, as you can query qemu_plugin_num_vcpus() to
know (dynamically) how many vcpus were created.
Yes, if the ids of dynamically initialized VCPUs are contiguous. I
wasn't sure they really are. And I distinctly remember we originally
used some query function and ended up with the maximum number of VCPUs
supported rather then those actually used. But that may have been
another function, or some unfortunate result of me being too cautious
and doing
It's a valid concern, and the good news is that they are guaranteed to
be contiguous.
In system mode, we initialize all vcpus *before* executing anything.
In user mode, they are spawned when new threads appear.
In reality, qemu_plugin_num_vcpus() returns the maximum number of vcpus
we had at some point (vs the real number of vcpus running).
It was introduced with scoreboards, to ensure the user can know how many
entries they contain, in a safe way.
Most of the usage we had for this was to allocate structures and collect
information per vcpu, especially at the end of execution (where some
vcpus will have exited already in user-mode). So choosing to return the
max is a valid abstraction.
| qemu_plugin_vcpu_for_each(id, vcpu_init);
in qemu_plugin_install.
And indeed, qemu_plugin_vcpu_for_each(id, vcpu_init) will only iterate
on active cpus.
+ break;
+ }
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+ g_autoptr(GString) report;
+ report = g_string_new("VCPU, interrupts, exceptions, hostcalls\n");
+ int vcpu;
+
+ for (vcpu = 0; vcpu < max_vcpus; vcpu++) {
vcpu < qemu_plugin_num_vcpus()
Yes, max_vcpus was introduced as an optimization. If we can rely on all
VCPUs with id < qemu_plugin_num_vcpus() having been active at some point
this becomes unnecessary.
+ qemu_plugin_register_vcpu_discon_cb(id, QEMU_PLUGIN_DISCON_TRAPS,
+ vcpu_discon);
+
The change from QEMU_PLUGIN_DISCON_TRAPS to QEMU_PLUGIN_DISCON_ALL should be
included in this patch, instead of next one.
Ah, thanks for pointing that out. I likely fumbled this at some point when
rebasing.
Regards,
Julian Ganz