| Message ID | 1393406718-16860-1-git-send-email-xbing6@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 26 February 2014 09:25, Xuebing Wang <xbing6@gmail.com> wrote: > This patch does below: > - Move the declaration of 2 translate functions from translate-all.h into > include/exec/exec-all.h > - remove file translate-all.h > > "translate-all.h" => "exec/exec-all.h" can be done by: > git grep -w "translate-all.h" | cut -d: -f1 | > xargs sed -i 's/\<translate-all.h\>/exec\/exec-all.h/g' > > Note: > 1) "exact whole word match" is considered. > 2) We may move translate related from include/exec/exec-all.h into > include/exec/translate.h later. Is there any particular benefit to this change? The function prototypes go from being in a header only included by the file that uses them to being in a header that's included by a lot of other code... thanks -- PMM
Il 26/02/2014 10:55, Peter Maydell ha scritto: > On 26 February 2014 09:25, Xuebing Wang <xbing6@gmail.com> wrote: >> This patch does below: >> - Move the declaration of 2 translate functions from translate-all.h into >> include/exec/exec-all.h >> - remove file translate-all.h >> >> "translate-all.h" => "exec/exec-all.h" can be done by: >> git grep -w "translate-all.h" | cut -d: -f1 | >> xargs sed -i 's/\<translate-all.h\>/exec\/exec-all.h/g' >> >> Note: >> 1) "exact whole word match" is considered. >> 2) We may move translate related from include/exec/exec-all.h into >> include/exec/translate.h later. > > Is there any particular benefit to this change? The function > prototypes go from being in a header only included by the file > that uses them to being in a header that's included by a lot > of other code... Indeed, that's the point of translate-all.h. Paolo
Hi Peter/Paolo, Thanks for replying. 1) I am wondering if some slight refactoring (in the sense of superficial change of moving things around) can make tcg/translationblock/cpu/exec/memory/softmmu more understandable, especially for new engineers. My goal is to explore the possibility of implementing tcg multi-threading, in a similar fashion of kvm multi-threading. It's still at early stage of a work-in-progress. As I am new to qemu code base, I spent some time trying to figure out the design. Examples are: -- What does exec (as in the filename of exec.c) really mean? -- What does "all" mean as in kvm-all,c (xen-all.c, translate-all.c)? I assume "all" means "target independent". But, aren't other files (e.g. block.c, gdbstub.c) target independent? -- What is the difference between "all" and "common" as in include/exec/cpu-all.h and cpu-common.h? In my local repository, I did some refactoring. Examples are: -- I renamed cpu-exec.c => tcg.c, to reflect the design that it's about one of the 3 accelerators (tcg, kvm, xen). -- I renamed cpus.c => vcpu.c (this can be tricky). My idea of the object hierarchy is: [ qom/Object ] [ qdev/DeviceState ] [ qom/CPUState ] [ vcpu API (or qom/vcpu) ] [ CPUArchState ] (in the other words, I am trying to abstract/model vcpu) -- I renamed include/sysemu/cpus.h => include/exec/vcpu.h -- And other changes. Examples: 1) cpu_exec_init_all() in exec.c is about memory, and its name is kind of confusing 2) include/exec/address-spaces.h says below, but "git grep -nw address-spaces.h" reveals a lot. /* * Internal interfaces between memory.c/exec.c/vl.c. Do not #include unless * you're one of them. */ I am wondering if we should slightly re-factor the organization of files, thus new engineers will be easier to understand the design about tcg/exec. 2) Theoretically, include/exec/exec-all.h should work when included from *ANY files*, even mistakenly, right? I hope this patch does NOT do any harm. In my local repository, I split TranslationBlock related from it to be another file include/translate.h. I will see what is the best way to "git send-email" these refactoring for the community to review, without causing too much confusion. The purpose is to make the design easier to understand for new engineers. Thanks again. On 02/26/2014 06:34 PM, Paolo Bonzini wrote: > Il 26/02/2014 10:55, Peter Maydell ha scritto: >> On 26 February 2014 09:25, Xuebing Wang <xbing6@gmail.com> wrote: >>> This patch does below: >>> - Move the declaration of 2 translate functions from >>> translate-all.h into >>> include/exec/exec-all.h >>> - remove file translate-all.h >>> >>> "translate-all.h" => "exec/exec-all.h" can be done by: >>> git grep -w "translate-all.h" | cut -d: -f1 | >>> xargs sed -i 's/\<translate-all.h\>/exec\/exec-all.h/g' >>> >>> Note: >>> 1) "exact whole word match" is considered. >>> 2) We may move translate related from include/exec/exec-all.h into >>> include/exec/translate.h later. >> >> Is there any particular benefit to this change? The function >> prototypes go from being in a header only included by the file >> that uses them to being in a header that's included by a lot >> of other code... > > Indeed, that's the point of translate-all.h. > > Paolo > >
diff --git a/exec.c b/exec.c index b69fd29..ec3890e 100644 --- a/exec.c +++ b/exec.c @@ -47,7 +47,7 @@ #include "exec/cpu-all.h" #include "exec/cputlb.h" -#include "translate-all.h" +#include "exec/exec-all.h" #include "exec/memory-internal.h" #include "exec/ram_addr.h" diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index a387922..f9afb7a 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -213,6 +213,10 @@ void tb_free(TranslationBlock *tb); void tb_flush(CPUArchState *env); void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr); +/* translate-all.c */ +void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len); +void tb_check_watchpoint(CPUArchState *env); + #if defined(USE_DIRECT_JUMP) #if defined(CONFIG_TCG_INTERPRETER) diff --git a/translate-all.c b/translate-all.c index 1ac0246..6379b1a 100644 --- a/translate-all.c +++ b/translate-all.c @@ -57,7 +57,7 @@ #endif #include "exec/cputlb.h" -#include "translate-all.h" +#include "exec/exec-all.h" #include "qemu/timer.h" //#define DEBUG_TB_INVALIDATE diff --git a/translate-all.h b/translate-all.h deleted file mode 100644 index f7e5932..0000000 --- a/translate-all.h +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Translated block handling - * - * Copyright (c) 2003 Fabrice Bellard - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, see <http://www.gnu.org/licenses/>. - */ -#ifndef TRANSLATE_ALL_H -#define TRANSLATE_ALL_H - -/* translate-all.c */ -void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len); -void cpu_unlink_tb(CPUState *cpu); -void tb_check_watchpoint(CPUArchState *env); - -#endif /* TRANSLATE_ALL_H */
This patch does below: - Move the declaration of 2 translate functions from translate-all.h into include/exec/exec-all.h - remove file translate-all.h "translate-all.h" => "exec/exec-all.h" can be done by: git grep -w "translate-all.h" | cut -d: -f1 | xargs sed -i 's/\<translate-all.h\>/exec\/exec-all.h/g' Note: 1) "exact whole word match" is considered. 2) We may move translate related from include/exec/exec-all.h into include/exec/translate.h later. Signed-off-by: Xuebing Wang <xbing6@gmail.com> --- exec.c | 2 +- include/exec/exec-all.h | 4 ++++ translate-all.c | 2 +- translate-all.h | 27 --------------------------- 4 files changed, 6 insertions(+), 29 deletions(-) delete mode 100644 translate-all.h