From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKi3E-0006OL-7J for qemu-devel@nongnu.org; Tue, 04 Mar 2014 00:38:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKi38-0004Oe-Iy for qemu-devel@nongnu.org; Tue, 04 Mar 2014 00:38:00 -0500 Received: from mail-oa0-x22f.google.com ([2607:f8b0:4003:c02::22f]:39975) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKi38-0004OP-BF for qemu-devel@nongnu.org; Tue, 04 Mar 2014 00:37:54 -0500 Received: by mail-oa0-f47.google.com with SMTP id i11so1778231oag.20 for ; Mon, 03 Mar 2014 21:37:53 -0800 (PST) Message-ID: <531566A6.7060302@gmail.com> Date: Tue, 04 Mar 2014 13:37:42 +0800 From: Xuebing wang MIME-Version: 1.0 References: <1393901250-3922-1-git-send-email-xbing6@gmail.com> <53154C75.9040506@suse.de> In-Reply-To: <53154C75.9040506@suse.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [Discussion 00/10] about API hierarchy List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= , qemu-devel@nongnu.org Cc: Paolo Bonzini , Eduardo Habkost , Stefan Hajnoczi Hi Andreas, thank you very much for your reply. Would you please help review/correct doc/api-hierarchy too? On 03/04/2014 11:45 AM, Andreas Färber wrote: > Hi Xuebing, > > Am 04.03.2014 03:47, schrieb Xuebing Wang: >> Q2) Does it make sense to remove NEED_CPU_H from qemu-common.h? > IMO not in this way. Work has been ongoing to obsolete NEED_CPU_H by > introducing CPUState in addition to CPUArchState and moving fields into > CPUState, so that less files need to include target-xxx/cpu.h. Your > approach seems to be rather spreading it to more and different files. IMHO, CPUArchState is NOT the only data structure that is target-specific, thus needs "cpu.h". target_ulong is a define, thus can NOT be moved into CPUState. And there are TARGET_LONG_BITS, TARGET_PAGE_BITS, etc More importantly, their derivatives (for lack of a better word) need "cpu.h" too. 'git grep -nw target_ulong include/' lists the derivatives of target_ulong. And there could be derivatives of the derivatives. After this patchset, excluding folders (bsd/user, disas/, hw/, include/hw, target-xxx/), only a few files need "cpu.h" > Please explain in more detail: Why is code being moved from qom/cpu.h to > vmstate.h for target-alpha? target alpha and openrisc use VMSTATE_CPU(), for alpha-linuxuser VMSTATE_CPU() uses vmstate_cpu_common and vmstate_dummy, and vmstate_dummy is declared in include/migration/vmstate.h. Currently, vmstate.h is NOT included in qom/cpu.h Considering API hierarchy (and their dependencies), I'd consider vmstate to depend on qom/cpu, rather than conversely. Do you agree? > Concerning gdbstub.h, have you investigated replacing remaining > CPUArchState usages with CPUState, like I started in a previous series? Yes, I did. I even tried to replace CPUArchState with CPUState for other subsystems, to make them architecture-independent. But, it involves changing too many places. :-) > > Concerning memory_mapping.h, have you checked the mailing list archives > for reasons why I didn't use my vaddr there myself? Pointing me to my > HACKING entry does not answer that. ;) (Either vaddr didn't exist yet at > the time or there was some reason not to use it, I would hope!) "target_ulong virt_addr" was there before the born of vaddr. vaddr was introduced in your commit 577f42c0e11a5bfb462ff3a217701cd5c4356fb4 dated Jul 6 2013. :-) At this moment, do you think we better use vaddr to make memory_mapping target-independent? > > An important thing to watch out for when moving includes around is > avoiding (more) circular dependencies. CC'ing Eduardo. Thanks for reminding. Yes, I keep avoiding circular dependencies in mind, that's the reason I wrote document docs/api-hierarchy. Do you agree that in general, strictly following api hierarchy is a good approach to avoid circular dependencies? > > Also note that I still haven't fully rebased my qom-cpu-13 branch yet, > which includes a number of field movements and is likely to clash with > some of your refactorings in CPU/exec/translate files. Thanks for reminding. > > Your observation is correct that not all things are in the best shape. > The question is in which way and in which order to address them. > To better judge, what are the immediate gains of your proposals? For > example, I don't spot any Makefile changes in your diffstat that benefit > from a dropped cpu.h include somewhere in terms of build dependencies. > Nor does your series include a proof of concept that something becomes > possible that wasn't before. Understanding the exact advantages would > help me in determining what priority to assign such changes. Thanks. My idea of the immediate gains is to circulate the api-hierarchy, and watch out dependencies while we adding new features. That's why I name this discussion "about API hierarchy" Here is a quote from Anthony's "kvm forum 2011" keynote (slide 3): We're growing so fast, and are so popular, that we simply don't have time to exercise and eat healthy. http://www.linux-kvm.org/wiki/images/5/57/2011-forum-qemu-keynote-liguori.pdf I hope api-hierarchy and removing NEED_CPU_H from qemu-common.h is the low-hanging fruit, in the sense of "exercise". :-) > > Regards, > Andreas > -- Thanks, Xuebing Wang