Re: [Qemu-devel] [Discussion 00/10] about API hierarchy

[Xuebing wang]

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