On Wed, Dec 09, 2020 at 07:11:40PM +0100, Philippe Mathieu-Daudé wrote: [...] > >>>> @@ -200,7 +199,7 @@ static void spapr_cpu_core_reset(DeviceState *dev) > >>>> int i; > >>>> > >>>> for (i = 0; i < cc->nr_threads; i++) { > >>>> - spapr_reset_vcpu(sc->threads[i]); > >>>> + spapr_reset_vcpu(sc->threads[i], sc->spapr); > >>> > >>> Why reset() needs access to the machine state, don't > >>> you have it in realize()? > >>> > >> > >> This is for the vCPU threads of the sPAPR CPU core. They don't have the > >> link to the machine state. > > > > They are created by spapr_create_vcpu() + spapr_realize_vcpu() in > > spapr_cpu_core_realize(), which has sc->spapr set... Am I missing > > something? > > Anyhow, from a QOM design point of view, resetfn() is not the correct > place to set a property IMHO, so Cc'ing Eduardo.
This patch is not setting the property on resetfn(), it is setting it on CPU core pre_plug(). This is more complex than simply using qdev_get_machine() and I don't see why it would be better, but I don't think it's wrong. -- Eduardo