Am 01.08.2012 20:25, schrieb Anthony Liguori: > Andreas Färber <afaer...@suse.de> writes: > >> Am 01.08.2012 17:43, schrieb Anthony Liguori: >>> Igor Mammedov <imamm...@redhat.com> writes: >>> >>>> v2: >>>> ommited moving of x86_cpu_realize() from cpu_x86_init() to pc_new_cpu(), >>>> to keep cpu_init implementation in -softmmu and -user targets the same >>>> in single place and maintanable. >>>> >>>> v3: >>>> reuse cpu_is_bsp() rather than open code check if apicbase has BSP bit >>>> set >>>> >>>> tree for testing: >>>> https://github.com/imammedo/qemu/tree/x86_reset_v3 >>>> >>>> comiple & run tested with x86_64-linux-user, x86_64-softmmu targets >>>> >>>> Igor Mammedov (2): >>>> target-i386: move cpu halted decision into x86_cpu_reset >>>> target-i386: move cpu_reset and reset callback to cpu.c >>> >>> Applied all. Thanks. >> >> So do you intend to refactor all machines accordingly or leave it >> inconsistent now? > > Are you asking me? > > No, I have no intention of touching any other machine. We're not going > to limit cleaning up target-i386 unless every other machine is cleaned > up too. > > Reset logic should live in the CPU. Seems like a no-brainer to me.
Yes, I'm asking you, since you replied and applied the series without responding to my review comment on patch 2/2. You probably applied it locally before reading my comments but then I would still have expected a reply on how to proceed in light of those comments: Before applying this, as I've pointed out to Igor at least once before, all machines do such reset handling themselves. Patch 2/2 that you applied makes target-i386 break away from that scheme. (I wonder that Peter hasn't protested yet...) Anyway, that being the last patch in this series, I see no value in doing this on its own for target-i386 only. So now we should either revert that patch and later replace it with one that does a touch-all change across the boards, or someone needs to volunteer (and you agree, during the Freeze) to refactor all other machines accordingly, which will take a while to get Acked-bys from machine maintainers... Or just defer touching reset callbacks until we have the CPU as a device and then drop the callbacks instead of moving them. Note the point of disagreement here is not "reset logic" - it's great that the APIC BSP fiddling is gone from PC with patch 1/2 - but the registration of system-level callbacks in cpu.c in patch 2/2. I thought we all agreed that we want to make CPU a device and have it reset as a device? No such callback in cpu.c will be needed then and we thus seem to be, in absence of follow-ups for 1.2, needlessly moving to-be-dead code around. Not doing that seems like a no-brainer to me. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
