From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52859) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Swulh-0003Qf-4o for qemu-devel@nongnu.org; Thu, 02 Aug 2012 08:44:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwulY-000690-RK for qemu-devel@nongnu.org; Thu, 02 Aug 2012 08:44:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42643) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwulY-00068v-I7 for qemu-devel@nongnu.org; Thu, 02 Aug 2012 08:44:36 -0400 Message-ID: <501A7630.1080705@redhat.com> Date: Thu, 02 Aug 2012 14:44:32 +0200 From: Igor Mammedov MIME-Version: 1.0 References: <1343855634-8779-1-git-send-email-afaerber@suse.de> <5019B159.5080900@suse.de> In-Reply-To: <5019B159.5080900@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-1.2] cpus: Register reset callback centrally List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: qemu-devel@nongnu.org, Anthony Liguori On 08/02/2012 12:44 AM, Andreas F=C3=A4rber wrote: > Am 01.08.2012 23:13, schrieb Andreas F=C3=A4rber: >> Despite repeated protest commit 65dee38052597b6285eb208125369f01b29ba6= c1 >> (target-i386: move cpu_reset and reset callback to cpu.c) moved >> registration of a reset callback from hw/pc.c to target-i386/cpu.c >> while all other CPU reset handlers were still registered from machines. >> >> Instead, improve the situation by registering the callback in >> qemu_init_vcpu(). Now new machines or CPUs no longer need to register >> their own callbacks, and we can revert the code in target-i386/cpu.c. >> >> Signed-off-by: Andreas F=C3=A4rber >> Cc: Anthony Liguori >> Cc: Igor Mammedov >> --- >> cpus.c | 8 ++++++++ >> target-i386/cpu.c | 11 ----------- >> 2 files changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index 756e624..f717a61 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -1020,6 +1020,13 @@ static void qemu_dummy_start_vcpu(CPUArchState = *env) >> } >> } >> >> +static void cpu_machine_reset(void *opaque) >> +{ >> + CPUState *cpu =3D opaque; >> + >> + cpu_reset(cpu); >> +} >> + >> void qemu_init_vcpu(void *_env) >> { >> CPUArchState *env =3D _env; >> @@ -1027,6 +1034,7 @@ void qemu_init_vcpu(void *_env) >> env->nr_cores =3D smp_cores; >> env->nr_threads =3D smp_threads; >> env->stopped =3D 1; >> + qemu_register_reset(cpu_machine_reset, ENV_GET_CPU(env)); >> if (kvm_enabled()) { >> qemu_kvm_start_vcpu(env); >> } else if (tcg_enabled()) { > [snip] > > Note that this is safe as long as qemu_init_vcpu() happens inside > cpu_foo_init(), the current convention. I believe Igor's APIC/BSP patch about happens inside cpu_foo_init(): could you explain why it's safe=20 only there? for x86 qemu_kvm_start_vcpu(env) is inside of realize() function, and in=20 case of using qdev_device_add() this call won't happen inside of=20 cpu_foo_init(). And in future with completely qomified CPUs we could probably get rid of=20 cpu_foo_init() and cpu_init() altogether. > that just got committed replaced an earlier attempt to change that. > Further reset handlers would then be registered after this one and IIUC > executed in registration order so that worst case cpu_reset() would be > executed multiple times if not dropped from machines' reset callbacks. > > What is missing here though is unregistration of the new reset callback > for future hot unplug. But waiting for comments before I drill some > qemu_finalize_vcpu() API callable from qom/cpu.c. > > Andreas >