[Qemu-devel] [PATCH] tcg arm/mips/ia64: add a comment about retranslation and caches

[Qemu-devel] [PATCH] tcg arm/mips/ia64: add a comment about retranslation and caches

[Aurelien Jarno]

Add a comment about cache coherency and retranslation, so that people
developping new targets based on existing ones are warned of the issue.

Cc: Alexander Graf <agraf@suse.de>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/arm/tcg-target.c  |    3 +++
 tcg/ia64/tcg-target.c |    3 +++
 tcg/mips/tcg-target.c |    4 +++-
 3 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 1eb5605..918e2f7 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -352,6 +352,9 @@ static inline void tcg_out_b(TCGContext *s, int cond, int32_t offset)
 
 static inline void tcg_out_b_noaddr(TCGContext *s, int cond)
 {
+    /* We pay attention here to not modify the branch target by skipping
+       the corresponding bytes. This ensure that caches and memory are
+       kept coherent during retranslation. */
 #ifdef HOST_WORDS_BIGENDIAN
     tcg_out8(s, (cond << 4) | 0x0a);
     s->code_ptr += 3;
diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
index e2e44f7..8dac7f7 100644
--- a/tcg/ia64/tcg-target.c
+++ b/tcg/ia64/tcg-target.c
@@ -871,6 +871,9 @@ static void tcg_out_br(TCGContext *s, int label_index)
 {
     TCGLabel *l = &s->labels[label_index];
 
+    /* We pay attention here to not modify the branch target by reading
+       the existing value and using it again. This ensure that caches and
+       memory are kept coherent during retranslation. */
     tcg_out_bundle(s, mmB,
                    tcg_opc_m48(TCG_REG_P0, OPC_NOP_M48, 0),
                    tcg_opc_m48(TCG_REG_P0, OPC_NOP_M48, 0),
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 4e92a50..e04b0dc 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -351,7 +351,9 @@ static inline void tcg_out_opc_imm(TCGContext *s, int opc, int rt, int rs, int i
  */
 static inline void tcg_out_opc_br(TCGContext *s, int opc, int rt, int rs)
 {
-    /* We need to keep the offset unchanged for retranslation */
+    /* We pay attention here to not modify the branch target by reading
+       the existing value and using it again. This ensure that caches and
+       memory are kept coherent during retranslation. */
     uint16_t offset = (uint16_t)(*(uint32_t *) s->code_ptr);
 
     tcg_out_opc_imm(s, opc, rt, rs, offset);
-- 
1.7.2.3

[Qemu-devel] Re: [PATCH] tcg arm/mips/ia64: add a comment about retranslation and caches

[Alexander Graf]

On 10.01.2011, at 18:34, Aurelien Jarno wrote:

> Add a comment about cache coherency and retranslation, so that people
> developping new targets based on existing ones are warned of the issue.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Thanks :)


Acked-by: Alexander Graf <agraf@suse.de>


Alex

[Qemu-devel] Re: [PATCH] tcg arm/mips/ia64: add a comment about retranslation and caches

[Edgar E. Iglesias]

On Mon, Jan 10, 2011 at 06:34:42PM +0100, Aurelien Jarno wrote:
> Add a comment about cache coherency and retranslation, so that people
> developping new targets based on existing ones are warned of the issue.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Looks good to me, thanks.

Acked-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>


> ---
>  tcg/arm/tcg-target.c  |    3 +++
>  tcg/ia64/tcg-target.c |    3 +++
>  tcg/mips/tcg-target.c |    4 +++-
>  3 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 1eb5605..918e2f7 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -352,6 +352,9 @@ static inline void tcg_out_b(TCGContext *s, int cond, int32_t offset)
>  
>  static inline void tcg_out_b_noaddr(TCGContext *s, int cond)
>  {
> +    /* We pay attention here to not modify the branch target by skipping
> +       the corresponding bytes. This ensure that caches and memory are
> +       kept coherent during retranslation. */
>  #ifdef HOST_WORDS_BIGENDIAN
>      tcg_out8(s, (cond << 4) | 0x0a);
>      s->code_ptr += 3;
> diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
> index e2e44f7..8dac7f7 100644
> --- a/tcg/ia64/tcg-target.c
> +++ b/tcg/ia64/tcg-target.c
> @@ -871,6 +871,9 @@ static void tcg_out_br(TCGContext *s, int label_index)
>  {
>      TCGLabel *l = &s->labels[label_index];
>  
> +    /* We pay attention here to not modify the branch target by reading
> +       the existing value and using it again. This ensure that caches and
> +       memory are kept coherent during retranslation. */
>      tcg_out_bundle(s, mmB,
>                     tcg_opc_m48(TCG_REG_P0, OPC_NOP_M48, 0),
>                     tcg_opc_m48(TCG_REG_P0, OPC_NOP_M48, 0),
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 4e92a50..e04b0dc 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -351,7 +351,9 @@ static inline void tcg_out_opc_imm(TCGContext *s, int opc, int rt, int rs, int i
>   */
>  static inline void tcg_out_opc_br(TCGContext *s, int opc, int rt, int rs)
>  {
> -    /* We need to keep the offset unchanged for retranslation */
> +    /* We pay attention here to not modify the branch target by reading
> +       the existing value and using it again. This ensure that caches and
> +       memory are kept coherent during retranslation. */
>      uint16_t offset = (uint16_t)(*(uint32_t *) s->code_ptr);
>  
>      tcg_out_opc_imm(s, opc, rt, rs, offset);
> -- 
> 1.7.2.3
>