On 3/3/20 3:19 PM, Philippe Mathieu-Daudé wrote:
On 3/3/20 11:05 AM, Peter Maydell wrote:
The CPUClass has a 'reset' method. This is a legacy from when
TYPE_CPU used not to inherit from TYPE_DEVICE. We don't need it any
more, as we can simply use the TYPE_DEVICE reset. The 'cpu_reset()'
function is kept as the API which most places use to reset a CPU; it
is now a wrapper which calls device_cold_reset() and then the
tracepoint function.
This change should not cause CPU objects to be reset more often
than they are at the moment, because:
* nobody is directly calling device_cold_reset() or
qdev_reset_all() on CPU objects
* no CPU object is on a qbus, so they will not be reset either
by somebody calling qbus_reset_all()/bus_cold_reset(), or
by the main "reset sysbus and everything in the qbus tree"
reset that most devices are reset by
Note that this does not change the need for each machine or whatever
to use qemu_register_reset() to arrange to call cpu_reset() -- that
is necessary because CPU objects are not on any qbus, so they don't
get reset when the qbus tree rooted at the sysbus bus is reset, and
this isn't being changed here.
All the changes to the files under target/ were made using the
included Coccinelle script, except:
(1) the deletion of the now-inaccurate and not terribly useful
"CPUClass::reset" comments was done with a perl one-liner afterwards:
perl -n -i -e '/ CPUClass::reset/ or print' target/*/*.c
(2) this bit of the s390 change was done by hand, because the
Coccinelle script is not sophisticated enough to handle the
parent_reset call being inside another function:
| @@ -96,8 +96,9 @@ static void s390_cpu_reset(CPUState *s,
cpu_reset_type type)
| S390CPU *cpu = S390_CPU(s);
| S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
| CPUS390XState *env = &cpu->env;
|+ DeviceState *dev = DEVICE(s);
|
|- scc->parent_reset(s);
|+ scc->parent_reset(dev);
| cpu->env.sigp_order = 0;
| s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
Testing was by 'make check' and 'make check-acceptance'.
I need this patch as a preliminary to some arm stuff I'm
doing, but I think it makes sense as a cleanup in its own
right so I'm sending it out early for review. If it's not
yet in master before I get round to finishing the stuff
that depends on it I'll resend it as part of that series.
Nice cleanup, thanks.
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
[...]
diff --git a/scripts/coccinelle/cpu-reset.cocci
b/scripts/coccinelle/cpu-reset.cocci
new file mode 100644
index 00000000000..396a724e514
--- /dev/null
+++ b/scripts/coccinelle/cpu-reset.cocci
@@ -0,0 +1,47 @@
+// Convert targets using the old CPUState reset to DeviceState reset
+//
+// Copyright Linaro Ltd 2020
+// This work is licensed under the terms of the GNU GPLv2 or later.
+//
+// spatch --macro-file scripts/cocci-macro-file.h \
+// --sp-file scripts/coccinelle/cpu-reset.cocci \
+// --keep-comments --smpl-spacing --in-place --include-headers
--dir target
+//
+// For simplicity we assume some things about the code we're modifying
+// that happen to be true for all our targets:
+// * all cpu_class_set_parent_reset() callsites have a 'DeviceClass
*dc' local
+// * the parent reset field in the target CPU class is 'parent_reset'
+// * no reset function already has a 'dev' local
+
+@@
+identifier cpu, x;
+typedef CPUState;
+@@
+struct x {
+...
+- void (*parent_reset)(CPUState *cpu);
++ DeviceReset parent_reset;
+...
+};
+@ rule1 @
+identifier resetfn;
+expression resetfield;
+identifier cc;
+@@
+- cpu_class_set_parent_reset(cc, resetfn, resetfield)
++ device_class_set_parent_reset(dc, resetfn, resetfield)
+@@
+identifier rule1.resetfn;
+identifier cpu, cc;
+typedef CPUState, DeviceState;
+@@
+-resetfn(CPUState *cpu)
+-{
++resetfn(DeviceState *dev)
++{
Nitpick: you don't need to include the bracket symbol in the diff:
@@
-resetfn(CPUState *cpu)
+resetfn(DeviceState *dev)
{
(simply indent it with a space).
++ CPUState *cpu = CPU(dev);
+<...
+- cc->parent_reset(cpu);
++ cc->parent_reset(dev);
+...>
+}
