Re: [PATCH 9/9] MAINTAINERS: replace Fabien by myself as Leon3 maintainer

2024-01-05 Thread Fabien Chouteau
Looks good Clement.

Reviewed-by: Fabien Chouteau 


-- 
Fabien Chouteau


Re: Support for Gaisler multicore LEONx SoCs

2022-08-22 Thread Fabien Chouteau
Hi all,

On Mon, Aug 1, 2022 at 6:42 PM Gregg Allison <
gregg.alli...@lasp.colorado.edu> wrote:

> Fabien, can I obtain the SMP Leon3/Leon4 fork from AdaCore directly?
>

Our fork is here: https://github.com/adacore/qemu/tree/qemu-stable-7.0.0
There is much more than SMP Leon3 in there :)

I add in copy Clément Chigot who is in charge of contribution to upstream
QEMU now.
I think we can put SMP Leon3 on top of the TODO list for contributions.
That being said, no promises on when it can be done :)

Regards,

-- 
Fabien Chouteau


Re: Support for Gaisler multicore LEONx SoCs

2022-07-20 Thread Fabien Chouteau
Hello everyone,

On Fri, Jul 8, 2022 at 12:16 PM Frederic Konrad 
wrote:

> About the SMP support AdaCore had a few patches for it, I'll let Fabien
> answer.
>

The patches for SMP support actually come from Gaisler originally (if I
remember correctly).

For sure we at AdaCore support SMP Leon3/Leon4 in our fork of QEMU, and I
think all the required patches are contributed upstream.

-- 
Fabien Chouteau


Re: [PATCH] MAINTAINERS: change Fred Konrad's email address

2022-03-30 Thread Fabien Chouteau
On Wed, Mar 30, 2022 at 2:31 PM Frederic Konrad  wrote:

> frederic.kon...@adacore.com and kon...@adacore.com will stop working
> starting
> 2022-04-01.
>
> Use my personal email instead.
>
>
Reviewed-by: Fabien Chouteau >

-- 
Fabien Chouteau


Re: gdbstub.c CPU clusters not handled for ThreadInfo packets

2020-06-05 Thread Fabien Chouteau
On Tue, Jun 2, 2020 at 7:26 PM Peter Maydell  wrote:
> When there's multiple clusters, by default gdb will show you all the CPUs in 
> the first
> cluster. Would the behaviour be better if the other cluster in
> this machine was the first (default) one?

For our use case yes it would be better. I will look to see if this is
doable with a small patch on our fork.

> Hmm. That's the behavior for 'target remote' but 'target extended-remote'
> ought to support multiple inferiors. What gdb version is this?

Force of habit, I made a fool of myself and used only 'target remote'.
Sorry for the noise here.

Thanks for your help,



Re: gdbstub.c CPU clusters not handled for ThreadInfo packets

2020-06-02 Thread Fabien Chouteau
Thank you Peter,

Le mar. 2 juin 2020 à 18:44, Peter Maydell  a écrit :
>
> By default gdb does not attach to all clusters (this is IIRC something
> that can't
> be influenced on the QEMU end);

You mean I cannot have QEMU attach all clusters by default?

> are you explicitly asking it to attach to the second cluster?

So far the behavior that we had was to see all the CPUs without asking
explicitly.
I want to go back to that behavior because the current situation is a
huge drawback for our users.

> https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg07035.html
> is a mailing list post from January which gives the gdb commands which
> ought to cause it to attach to the second cluster on this machine.

The commands:
target extended-remote :1234
add-inferior
inferior 2
attach 2
set schedule-multiple
info threads

Do not work for me, "attach 2" in GDB asks me to kill the debugged program.

Thanks for your help,



gdbstub.c CPU clusters not handled for ThreadInfo packets

2020-06-02 Thread Fabien Chouteau
Hi all,

(Luc, I am putting in copy because from the logs it looks like you
implemented the multiprocess support)

Using the sifive_u machine on qemu-system-riscv64 which has 2 clusters of 1
and 4 CPUs respectively, when I use the "info threads" command in GDB only
one thread is displayed.
This thread is the only CPU of the 1st cluster, the CPUs of cluster 2nd are
not listed at all.

Looking at the GDB remote packets (see below) we can see that indeed only
one thread is listed (mp01.01). It seems to me that the functions
handle_query_first_threads() and handle_query_threads() of gdbstub.c are
not handling the new multiprocess implementation. I would like to fix it
but I am lost between cpus, processes, threads, etc.

Does anyone have an idea how to fix this?

Regards,

(gdb) info threads
Sending packet: $qfThreadInfo#bb...Ack
Packet received: mp01.01
Sending packet: $qsThreadInfo#c8...Ack
Packet received: l
Sending packet: $qThreadExtraInfo,p1.1#85...Ack
Packet received:
7369666976652d6535312d72697363762d6370752068617274735b305d205b72756e6e696e675d
  Id   Target IdFrame
* 1Thread 1.1 (sifive-e51-riscv-cpu harts[0] [running])
0x1000 in ?? ()
(gdb)


Re: LEON3 networking

2019-10-16 Thread Fabien Chouteau
Hello people,

On 15/10/2019 18:57, Philippe Mathieu-Daudé wrote:
> Hi Joshua,
> 
> On 10/15/19 3:17 PM, Joshua Shaffer wrote:
>> Hello,
>>
>> I've been using the LEON3 port of qemu, and am wondering if anyone has 
>> touched the networking setup for such since the thread here: 
>> https://lists.rtems.org/pipermail/users/2014-September/028224.html
> 
> Thanks for sharing this!
> 
> Good news, Jiri keeps rebasing his patch with the latest stable version.
> Bad news, he didn't not signed his work with a "Signed-off-by" tag so we can 
> not take this as it into the mainstream repository, see 
> https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
> 

The Gaisler patches have been rewrote by my colleague Frederic (in CC) and they 
are now in mainstream.
(see https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03869.html)

But none of them are implementing network support, and I never heard of someone 
working on network for leon3.

Regards,




Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

2019-08-05 Thread Fabien Chouteau
On 05/08/2019 18:10, Bin Meng wrote:
> Thank you for the suggestion. A patch was created for this:
> http://patchwork.ozlabs.org/patch/1142282/

Awesome, thank you Bin!




Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

2019-07-08 Thread Fabien Chouteau
Hi Bin,

Thanks for this patch.

I know I am very late to the game but I have a comment here.

On 17/05/2019 17:51, Bin Meng wrote:
> +/* create PLIC hart topology configuration string */
> +plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * 
> smp_cpus;
> +plic_hart_config = g_malloc0(plic_hart_config_len);
> +for (i = 0; i < smp_cpus; i++) {
> +if (i != 0) {
> +strncat(plic_hart_config, ",", plic_hart_config_len);
> +}
> +strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> +plic_hart_config_len);
> +plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> +}
> +

This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the 
PLICs are M,MS,MS,MS,MS because of the monitor hart #0.

This means a different memory layout than the real hardware.

For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in 
QEMU, instead of #1 M-Mode interrupt enables for the real hardware.

To fix this I suggest to change this loop to:

for (i = 0; i < smp_cpus; i++) {
if (i != 0) {
strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
plic_hart_config_len);
} else {
strncat(plic_hart_config, "M", plic_hart_config_len);
}
plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
}

This will make hart #0 PLIC in M mode and the others in MS.

What do you think?

Best regards,



Re: [Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

2019-06-17 Thread Fabien Chouteau
On 14/06/2019 14:10, Palmer Dabbelt wrote:
> Sorry this took a while to fix, I've just sent a patch to fix the memory leak.

Thank you for taking care of this!



Re: [Qemu-devel] [PATCH V1 1/3] leon3: add a little bootloader

2019-04-23 Thread Fabien Chouteau
On 19/04/2019 12:18, KONRAD Frederic wrote:
> This adds a little bootloader to the leon3_machine when a ram image is
> given through the kernel parameter and no bios are provided:
>   * The UART transmiter is enabled.
>   * The TIMER is initialized.
> 
> Signed-off-by: KONRAD Frederic 

Reviewed-by: Fabien Chouteau 



Re: [Qemu-devel] [PATCH V1 2/3] leon3: introduce the plug and play mecanism

2019-04-23 Thread Fabien Chouteau
On 19/04/2019 12:18, KONRAD Frederic wrote:
> This adds the AHB and APB plug and play devices.
> They are scanned during the linux boot to discover the various peripheral.
> 
> Signed-off-by: KONRAD Frederic 

Reviewed-by: Fabien Chouteau 




Re: [Qemu-devel] [PATCH V1 3/3] MAINTAINERS: add myself for leon3

2019-04-23 Thread Fabien Chouteau
On 19/04/2019 12:18, KONRAD Frederic wrote:
> Signed-off-by: KONRAD Frederic 

Reviewed-by: Fabien Chouteau 



Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device

2019-03-26 Thread Fabien Chouteau
Hi Palmer,

On 26/03/2019 09:58, Palmer Dabbelt wrote:
> Do you have anything that actually glues this to a machine so I can test it?
> 

In this patch I do instantiate the device in sifive_e machine.

Regards,



Re: [Qemu-devel] [PATCH] RISC-V: fix single stepping over ret and other branching instructions

2019-03-25 Thread Fabien Chouteau
Thanks Richard,

I sent a version 2.

On 22/03/2019 16:24, Richard Henderson wrote:
> On 3/22/19 4:22 AM, Fabien Chouteau wrote:
>> +/* Wrapper around tcg_gen_exit_tb that handles single stepping */
>> +static void exit_tb(DisasContext *ctx, TranslationBlock *tb, unsigned idx)
>> +{
>> +if (ctx->base.singlestep_enabled) {
>> +gen_exception_debug();
>> +} else {
>> +tcg_gen_exit_tb(tb, idx);
>> +}
>> +}
> 
> You should remove the TB and idx parameters here and pass NULL, 0 to
> tcg_gen_exit_tb.
> 
>> @@ -138,14 +158,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
>> target_ulong dest)
>>  /* chaining is only allowed when the jump is to the same page */
>>  tcg_gen_goto_tb(n);
>>  tcg_gen_movi_tl(cpu_pc, dest);
>> -tcg_gen_exit_tb(ctx->base.tb, n);
>> +exit_tb(ctx, ctx->base.tb, n);
> 
> Because this is the only non-zero use, and it is already protected by
> use_goto_tb, which includes the single-step check.
> 
> Because goto_tb(n) must be paired with exit_tb(tb, n), and vice-versa.
> 
> 
> r~
> 




[Qemu-devel] [PATCH V2] RISC-V: fix single stepping over ret and other branching instructions

2019-03-25 Thread Fabien Chouteau
This patch introduces wrappers around the tcg_gen_exit_tb() and
tcg_gen_lookup_and_goto_ptr() functions that handle single stepping,
i.e. call gen_exception_debug() when single stepping is enabled.

Theses functions are then used instead of the originals, bringing single
stepping handling in places where it was previously ignored such as jalr
and system branch instructions (ecall, mret, sret, etc.).

Signed-off-by: Fabien Chouteau 
---
 .../riscv/insn_trans/trans_privileged.inc.c   |  8 ++---
 target/riscv/insn_trans/trans_rvi.inc.c   |  6 ++--
 target/riscv/translate.c  | 30 +++
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/target/riscv/insn_trans/trans_privileged.inc.c 
b/target/riscv/insn_trans/trans_privileged.inc.c
index acb605923e..664d6ba3f2 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -22,7 +22,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 {
 /* always generates U-level ECALL, fixed in do_interrupt handler */
 generate_exception(ctx, RISCV_EXCP_U_ECALL);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -30,7 +30,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
 {
 generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -47,7 +47,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 
 if (has_ext(ctx, RVS)) {
 gen_helper_sret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 } else {
 return false;
@@ -68,7 +68,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 #ifndef CONFIG_USER_ONLY
 tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
 gen_helper_mret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 #else
diff --git a/target/riscv/insn_trans/trans_rvi.inc.c 
b/target/riscv/insn_trans/trans_rvi.inc.c
index d420a4d8b2..37b0b9dd19 100644
--- a/target/riscv/insn_trans/trans_rvi.inc.c
+++ b/target/riscv/insn_trans/trans_rvi.inc.c
@@ -60,7 +60,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
 if (a->rd != 0) {
 tcg_gen_movi_tl(cpu_gpr[a->rd], ctx->pc_succ_insn);
 }
-tcg_gen_lookup_and_goto_ptr();
+lookup_and_goto_ptr(ctx);
 
 if (misaligned) {
 gen_set_label(misaligned);
@@ -483,7 +483,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
  * however we need to end the translation block
  */
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
-tcg_gen_exit_tb(NULL, 0);
+exit_tb(ctx);
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -504,7 +504,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 gen_io_end(); \
 gen_set_gpr(a->rd, dest); \
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); \
-tcg_gen_exit_tb(NULL, 0); \
+exit_tb(ctx); \
 ctx->base.is_jmp = DISAS_NORETURN; \
 tcg_temp_free(source1); \
 tcg_temp_free(csr_store); \
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 049fa65c66..7417c532f2 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -109,6 +109,26 @@ static void gen_exception_debug(void)
 tcg_temp_free_i32(helper_tmp);
 }
 
+/* Wrapper around tcg_gen_exit_tb that handles single stepping */
+static void exit_tb(DisasContext *ctx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_exit_tb(NULL, 0);
+}
+}
+
+/* Wrapper around tcg_gen_lookup_and_goto_ptr that handles single stepping */
+static void lookup_and_goto_ptr(DisasContext *ctx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
+}
+
 static void gen_exception_illegal(DisasContext *ctx)
 {
 generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
@@ -138,14 +158,14 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
target_ulong dest)
 /* chaining is only allowed when the jump is to the same page */
 tcg_gen_goto_tb(n);
 tcg_gen_movi_tl(cpu_pc, dest);
+
+/* No need to check for single stepping here as use_goto_tb() will
+ * return false in case of single stepping.
+ */
 tcg_gen_exit_tb(ctx->base.tb, n);
 } else {
 tcg_gen_movi_tl(cpu_pc, dest);
-if (ctx->base.singlestep_enabled) {
-gen_exception_debug();
-  

[Qemu-devel] [PATCH] RISC-V: fix single stepping over ret and other branching instructions

2019-03-22 Thread Fabien Chouteau
This patch introduces wrappers around the tcg_gen_exit_tb() and
tcg_gen_lookup_and_goto_ptr() functions that handle single stepping,
i.e. call gen_exception_debug() when single stepping is enabled.

Theses functions are then used instead of the originals, bringing single
stepping handling in places where it was previously ignored such as jalr
and system branch instructions (ecall, mret, sret, etc.).

Signed-off-by: Fabien Chouteau 
---
 .../riscv/insn_trans/trans_privileged.inc.c   |  8 +++---
 target/riscv/insn_trans/trans_rvi.inc.c   |  6 ++--
 target/riscv/translate.c  | 28 +++
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/target/riscv/insn_trans/trans_privileged.inc.c 
b/target/riscv/insn_trans/trans_privileged.inc.c
index acb605923e..fe660ab3dd 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -22,7 +22,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 {
 /* always generates U-level ECALL, fixed in do_interrupt handler */
 generate_exception(ctx, RISCV_EXCP_U_ECALL);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -30,7 +30,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
 {
 generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -47,7 +47,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 
 if (has_ext(ctx, RVS)) {
 gen_helper_sret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 } else {
 return false;
@@ -68,7 +68,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 #ifndef CONFIG_USER_ONLY
 tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
 gen_helper_mret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 #else
diff --git a/target/riscv/insn_trans/trans_rvi.inc.c 
b/target/riscv/insn_trans/trans_rvi.inc.c
index d420a4d8b2..44ae573768 100644
--- a/target/riscv/insn_trans/trans_rvi.inc.c
+++ b/target/riscv/insn_trans/trans_rvi.inc.c
@@ -60,7 +60,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
 if (a->rd != 0) {
 tcg_gen_movi_tl(cpu_gpr[a->rd], ctx->pc_succ_insn);
 }
-tcg_gen_lookup_and_goto_ptr();
+lookup_and_goto_ptr(ctx);
 
 if (misaligned) {
 gen_set_label(misaligned);
@@ -483,7 +483,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
  * however we need to end the translation block
  */
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
-tcg_gen_exit_tb(NULL, 0);
+exit_tb(ctx, NULL, 0);
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -504,7 +504,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 gen_io_end(); \
 gen_set_gpr(a->rd, dest); \
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); \
-tcg_gen_exit_tb(NULL, 0); \
+exit_tb(ctx, NULL, 0); \
 ctx->base.is_jmp = DISAS_NORETURN; \
 tcg_temp_free(source1); \
 tcg_temp_free(csr_store); \
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 049fa65c66..c438400b19 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -109,6 +109,26 @@ static void gen_exception_debug(void)
 tcg_temp_free_i32(helper_tmp);
 }
 
+/* Wrapper around tcg_gen_exit_tb that handles single stepping */
+static void exit_tb(DisasContext *ctx, TranslationBlock *tb, unsigned idx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_exit_tb(tb, idx);
+}
+}
+
+/* Wrapper around tcg_gen_lookup_and_goto_ptr that handles single stepping */
+static void lookup_and_goto_ptr(DisasContext *ctx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
+}
+
 static void gen_exception_illegal(DisasContext *ctx)
 {
 generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
@@ -138,14 +158,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
target_ulong dest)
 /* chaining is only allowed when the jump is to the same page */
 tcg_gen_goto_tb(n);
 tcg_gen_movi_tl(cpu_pc, dest);
-tcg_gen_exit_tb(ctx->base.tb, n);
+exit_tb(ctx, ctx->base.tb, n);
 } else {
 tcg_gen_movi_tl(cpu_pc, dest);
-if (ctx->base.singlestep_enabled) {
-gen_exception_debug();
-} else {
-tcg_gen_lookup_a

Re: [Qemu-devel] [Qemu-riscv] [PATCH] SiFive RISC-V GPIO Device

2019-02-13 Thread Fabien Chouteau
On 13/02/2019 01:06, Alistair Francis wrote:> Thanks for the patch!
You are welcome!

Let me know if there is something to improve.

Regads,



[Qemu-devel] [PATCH] SiFive RISC-V GPIO Device

2019-02-12 Thread Fabien Chouteau
QEMU model of the GPIO device on the SiFive E300 series SOCs.

The pins are not used by a board definition yet, however this
implementation can already be used to trigger GPIO interrupts from the
software by configuring a pin as both output and input.

Signed-off-by: Fabien Chouteau 
---
 Makefile.objs  |   1 +
 hw/riscv/Makefile.objs |   1 +
 hw/riscv/sifive_e.c|  28 ++-
 hw/riscv/sifive_gpio.c | 388 +
 hw/riscv/trace-events  |   7 +
 include/hw/riscv/sifive_e.h|   8 +-
 include/hw/riscv/sifive_gpio.h |  72 ++
 7 files changed, 501 insertions(+), 4 deletions(-)
 create mode 100644 hw/riscv/sifive_gpio.c
 create mode 100644 hw/riscv/trace-events
 create mode 100644 include/hw/riscv/sifive_gpio.h

diff --git a/Makefile.objs b/Makefile.objs
index 67a054b08a..d40eb089ae 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -184,6 +184,7 @@ trace-events-subdirs += hw/virtio
 trace-events-subdirs += hw/watchdog
 trace-events-subdirs += hw/xen
 trace-events-subdirs += hw/gpio
+trace-events-subdirs += hw/riscv
 trace-events-subdirs += io
 trace-events-subdirs += linux-user
 trace-events-subdirs += migration
diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs
index 1dde01d39d..ced7935371 100644
--- a/hw/riscv/Makefile.objs
+++ b/hw/riscv/Makefile.objs
@@ -7,5 +7,6 @@ obj-y += sifive_plic.o
 obj-y += sifive_test.o
 obj-y += sifive_u.o
 obj-y += sifive_uart.o
+obj-y += sifive_gpio.o
 obj-y += spike.o
 obj-y += virt.o
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 5d9d65ff29..49c1dd986c 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -146,11 +146,15 @@ static void riscv_sifive_e_soc_init(Object *obj)
 _abort);
 object_property_set_int(OBJECT(>cpus), smp_cpus, "num-harts",
 _abort);
+sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0",
+  >gpio, sizeof(s->gpio),
+  TYPE_SIFIVE_GPIO);
 }
 
 static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
 const struct MemmapEntry *memmap = sifive_e_memmap;
+Error *err = NULL;
 
 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
@@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
 memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
 sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
-sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
-memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
+
+/* GPIO */
+
+object_property_set_bool(OBJECT(>gpio), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+/* Map GPIO registers */
+sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0, memmap[SIFIVE_E_GPIO0].base);
+
+/* Pass all GPIOs to the SOC layer so they are available to the board */
+qdev_pass_gpios(DEVICE(>gpio), dev, NULL);
+
+/* Connect GPIO interrupts to the PLIC */
+for (int i = 0; i < 32; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), i,
+   qdev_get_gpio_in(DEVICE(s->plic),
+SIFIVE_E_GPIO0_IRQ0 + i));
+}
+
 sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base,
 serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ));
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0",
diff --git a/hw/riscv/sifive_gpio.c b/hw/riscv/sifive_gpio.c
new file mode 100644
index 00..06bd8112d7
--- /dev/null
+++ b/hw/riscv/sifive_gpio.c
@@ -0,0 +1,388 @@
+/*
+ * sifive System-on-Chip general purpose input/output register definition
+ *
+ * Copyright 2019 AdaCore
+ *
+ * Base on nrf51_gpio.c:
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/riscv/sifive_gpio.h"
+#include "trace.h"
+
+static void update_output_irq(SIFIVEGPIOState *s)
+{
+
+uint32_t pending;
+uint32_t pin;
+
+pending = s->high_ip & s->high_ie;
+pending |= s->low_ip & s->low_ie;
+pending |= s->rise_ip & s->rise_ie;
+pending |= s->fall_ip & s->fall_ie;
+
+for (int i = 0; i < SIFIVE_GPIO_PINS; i++) {
+pin = 1 << i;
+qemu_set_irq(s->irq[i], (pending & pin) != 0);
+trace_sifive_gpio_update_output_irq(i, (pending & pin) != 0);
+}
+}
+
+static void update_state(SIFIVEGPIOState *s)
+{
+size_t i;
+bool prev_ival, in, in_mask, port, out_xor, pull, output_en, input_en,
+rise_ip, fall_ip, low_ip, high

Re: [Qemu-devel] [PATCH] hw/riscv/sifive_clint.c: avoid integer overflow in timecmp write

2019-02-07 Thread Fabien Chouteau
Hello Alistair,

On 07/02/2019 01:42, Alistair Francis wrote:> 
> Can you describe what this fixes?
> 

I encountered this problem when I tried to write 0x in timecmp.

With the integer overflow in QEMU, writing this value means that the QEMU timer
will be set in the past.

> Won't an overflow be ok as we then just wrap around anyway? I guess
> there is a problem if we want a value so large that we wrap around
> past our current time though.
> 

The overflow was in the computation of the value `next_ns`. It is used to set
the QEMU timer:

timer_mod(cpu->env.timer, next_ns);

A negative `next_ns` -because of the overflow- means that the timer
triggers immediately instead of far in the future.

Regards,



[Qemu-devel] [PATCH] hw/riscv/sifive_clint.c: avoid integer overflow in timecmp write

2019-01-30 Thread Fabien Chouteau
Writing a high value in timecmp leads to an integer overflow. This patch
modifies the code to detect such case, and use the maximum integer value
as the next trigger for the timer.

Signed-off-by: Fabien Chouteau 
---
 hw/riscv/sifive_clint.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index d4c159e937..1ca1f8c75e 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -38,8 +38,10 @@ static uint64_t cpu_riscv_read_rtc(void)
  */
 static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value)
 {
-uint64_t next;
 uint64_t diff;
+uint64_t lapse_ns;
+uint64_t clock_ns;
+int64_t  next_ns;
 
 uint64_t rtc_r = cpu_riscv_read_rtc();
 
@@ -54,10 +56,29 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, 
uint64_t value)
 /* otherwise, set up the future timer interrupt */
 riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0));
 diff = cpu->env.timecmp - rtc_r;
-/* back to ns (note args switched in muldiv64) */
-next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-muldiv64(diff, NANOSECONDS_PER_SECOND, SIFIVE_CLINT_TIMEBASE_FREQ);
-timer_mod(cpu->env.timer, next);
+
+/*
+ * How many nanoseconds until the next trigger (note args switched in
+ * muldiv64)
+ */
+lapse_ns = muldiv64(diff,
+NANOSECONDS_PER_SECOND,
+SIFIVE_CLINT_TIMEBASE_FREQ);
+
+/* Current time in nanoseconds */
+clock_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+if ((G_MAXINT64 - clock_ns) <= lapse_ns) {
+/*
+ * clock + lapse would overflow on 64bit. The highest 64bit value is
+ * used as the next trigger time.
+ */
+next_ns = G_MAXINT64;
+} else {
+next_ns = clock_ns + lapse_ns;
+}
+
+timer_mod(cpu->env.timer, next_ns);
 }
 
 /*
-- 
2.17.1




Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-25 Thread Fabien Chouteau
On 24/01/2018 20:51, Mark Cave-Ayland wrote:
> On 11/01/18 11:48, Fabien Chouteau wrote:
> 
>> On 10/01/2018 21:43, Jean-Christophe Dubois wrote:
>>> With the LEON3 IRQ controller IRQs can be acknoledged 2 ways:
>>> * Explicitely by software writing to the CLEAR_OFFSET register
>>> * Implicitely when the procesor is done running the trap handler attached
>>>    to the IRQ.
>>>
>>
>> Thanks Jean-Christophe,
>>
>> I tested the patch with our software and it works.
>>
>> Reviewed-by: Fabien Chouteau <chout...@adacore.com>
> 
> Thanks. I've applied this (with some minor touch-ups to the commit message) 
> to my qemu-sparc branch.
> 

Thanks you Mark!




Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-16 Thread Fabien Chouteau
On 15/01/2018 18:27, Jean-Christophe Dubois wrote:
> Le 2018-01-15 14:45, Jean-Christophe Dubois a écrit :
>>
>> Note: For now I am not so much interested in the AMBA discovery as the
>> type of software platform I am thinking about is embedded where the
>> hardware is well known ahead of time. This discovery capability would
>> make sense for more generic OS like linux or such. We cannot require
>> all embedded OS to implement the AMBA discovery process.
> 
> For the configurability of Qemu, I was thinking of using something similar to 
> the Qemu provided by Xilinx (http://www.wiki.xilinx.com/QEMU). Basically, you 
> provide a DTB file as a Qemu command line argument and Qemu will build the 
> various devices (including addresses and interrupts) based on the content of 
> this file. Then when running an OS, it can provide the DTB file (for example 
> to Linux) that match exactly the emulated platform.
> 
> This should allow to build "any" variation of the platform and to add devices 
> as you need them. This makes sense for Xilinx (their customer are building 
> custom platforms) and it would also make sense for LEON as the CPU core is 
> usually integrated inside a custom SOC/FPGA.
> 

Makes sense, another intermediate solution is to add an argument to the
function leon3_generic_hw_init(), A struct that specifies the address of
the peripherals. That way it will be easy to add more machine definition
without bringing more maintenance burden.




Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-16 Thread Fabien Chouteau
On 15/01/2018 19:16, Mark Cave-Ayland wrote:
> On 11/01/18 11:48, Fabien Chouteau wrote:
> 
>> On 10/01/2018 21:43, Jean-Christophe Dubois wrote:
>>> With the LEON3 IRQ controller IRQs can be acknoledged 2 ways:
>>> * Explicitely by software writing to the CLEAR_OFFSET register
>>> * Implicitely when the procesor is done running the trap handler attached
>>>    to the IRQ.
>>>
>>
>> Thanks Jean-Christophe,
>>
>> I tested the patch with our software and it works.
>>
>> Reviewed-by: Fabien Chouteau <chout...@adacore.com>
> 
> Thanks Fabien. Does that mean you would like me to take this patch for my 
> qemu-sparc branch?
> 

Yes please :)

Should I use Acked-by?



Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-15 Thread Fabien Chouteau
On 12/01/2018 15:10, Jean-Christophe Dubois wrote:
> Le 2018-01-12 11:55, Fabien Chouteau a écrit :
>> On 11/01/2018 13:35, Jean-Christophe Dubois wrote:
>>> Thanks Fabien,
>>>
>>> Now, as a side question, could you tell me which reference LEON3 platform 
>>> is implemented by Qemu in leon3_generic?
>>>
>>
>> I think it was the based on the FPGA version of Leon3 I was using at the
>> time. The name leon3_generic comes from my will to make it a
>> configurable board where users could define the number and the location
>> of the different peripherals, I never had time to work on this.
> 
> I see. I am not sure how to bring configurability to Qemu. There is the 
> possibility to describe the hw PTF with DTC/DTB or something similar. I think 
> some people were working on it for the ARM Qemu platform (but I am not sure 
> what happened to this initiative).
> 
> Now in the meantime, would it make sense to move leon3_generic to a tsim 
> compatible platform?
> 

I don't think so, leon3_generic is compatible with a real hardware which
is also interesting for comparison.

> This would allow to validate the same software on the 2 simulators (obviously 
> it would not be compatible with your specific FPGA version for now).
>

The Leon3 AMBA bus provides a way to discover the peripherals and their
address, so any system should be capable of supporting different
peripheral layouts.

Here's an example of AMBA discovery code from a very old project of mine
(don't judge me on this :) :
https://github.com/Fabien-Chouteau/kabitbol/blob/master/src/amba.c

There was a couple of patches submitted some times ago to add Leon3 AMBA
support in QEMU, I think it's time to bring them back...




Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-12 Thread Fabien Chouteau
On 11/01/2018 13:35, Jean-Christophe Dubois wrote:
> Thanks Fabien,
> 
> Now, as a side question, could you tell me which reference LEON3 platform is 
> implemented by Qemu in leon3_generic?
> 

I think it was the based on the FPGA version of Leon3 I was using at the
time. The name leon3_generic comes from my will to make it a
configurable board where users could define the number and the location
of the different peripherals, I never had time to work on this.



Re: [Qemu-devel] [PATCH] LEON3 IRQMP: Fix IRQ software ack

2018-01-11 Thread Fabien Chouteau
On 10/01/2018 21:43, Jean-Christophe Dubois wrote:
> With the LEON3 IRQ controller IRQs can be acknoledged 2 ways:
> * Explicitely by software writing to the CLEAR_OFFSET register
> * Implicitely when the procesor is done running the trap handler attached
>   to the IRQ.
> 

Thanks Jean-Christophe,

I tested the patch with our software and it works.

Reviewed-by: Fabien Chouteau <chout...@adacore.com>



Re: [Qemu-devel] [SPARC] question on LEON IRQMP interrupt controller.

2018-01-02 Thread Fabien Chouteau
Hello Jean-Christophe,

I'm the original author of this patch and I add in copy my colleague
Frederic.

On 02/01/2018 12:13, Jean-Christophe DUBOIS wrote:
> I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct
> when it comes to acknowledging interrupts.
>
> With the actual code an interrupt can be lowered/acked only by an
> "ack" from the processor which means that the trap handler related to
> this external interrupt needs to be run for the ack to happen.
>
> In particular this means that the interrupt cannot be acked only by
> software. Even if the software clears the "pending" interrupts (by
> writing to the CLEAR_OFFSET register before the interrupt handler is
> run) this does not clear the interrupt to the processor (which is kept
> asserted until the handler is run and the interrupt acked by the
> processor). Do you know if this is indeed the intended behavior (I
> understand that for most operating system the interrupt handler will
> be run at last and this does not make a difference)?
>
> I would expect that clearing interrupt through software (by writing to
> the CLEAR_OFFSET register) would have the same effect as the processor
> acknowledgment (and could avoid to run the interrupt handler if things
> have already been taken care of by software).
>
> Unfortunately the documentation I got (on the web) on the IRQMP is not
> very clear on the topic.
>

I don't remember all the details of this CPU on top of my head, I worked
on this years ago.

If you have access to a real board the best would be to compare the
behavior of the CPU on it. There's also a cycle accurate simulator of
Leon3, you can download an evaluation version here:
http://www.gaisler.com/index.php/downloads/simulators


> Anyway you can find below the patch I'd like to provide for IRQMP.
>

Can you explain the reason for this change? Why can't you use the
current interrupt handling?

Regards,




Re: [Qemu-devel] LEON3 timer patch

2017-04-03 Thread Fabien Chouteau
On 30/03/2017 21:30, Gabriele Galeotti wrote:
> 
> Hi all.
> According to "GR712RC Dual-Core LEON3FT SPARC V8 Processor User’s Manual",
> "11.3 Registers", pg 87-88, Table 55 Timer control register, the IP 
> "interrupt pending"
> bit:
> 
> Interrupt Pending (IP): The core sets this bit to ‘1’ when an interrupt is 
> signalled. This bit remains ‘1’
> until cleared by writing ‘0’ to this bit.
> 
> Thus  the code handling should changed so that the pending bit is mantained 
> when "value" has a 1
> in that position.

Thanks Gabriele,

I think my implementation is based on the "GRLIB IP Core User' Manual"
which says "Interrupt Pending (IP): The core sets this bit to "1" when
an interrupt is signalled. This bit remains "1" until cleared by writing
"1" to this bit, writes of "0" have no effect."

Do you know why the two docs are different on that aspect?


> 
> Signed-off-by: Gabriele Galeotti 
> ---
>  hw/timer/grlib_gptimer.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
> index 4ed96e9..c555ae8 100644
> --- a/hw/timer/grlib_gptimer.c
> +++ b/hw/timer/grlib_gptimer.c
> @@ -276,9 +276,6 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,
>  trace_grlib_gptimer_writel(id, addr, value);
> .
>  if (value & GPTIMER_INT_PENDING) {
> -/* clear pending bit */
> -value &= ~GPTIMER_INT_PENDING;
> -} else {
>  /* keep pending bit */
>  value |= unit->timers[id].config & GPTIMER_INT_PENDING;
>  }

My understanding is that with your modifications, users can set the
pending bit by writing one to it.


I suggest something like this:

diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
index a05304d..4344787 100644
--- a/hw/timer/grlib_gptimer.c
+++ b/hw/timer/grlib_gptimer.c
@@ -275,11 +275,14 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,
 case CONFIG_OFFSET:
 trace_grlib_gptimer_writel(id, addr, value);

+/* Interrupt pending. Remains 1 until cleared by writing 0 to this
+ * bit.
+ */
 if (value & GPTIMER_INT_PENDING) {
-/* clear pending bit */
+/* Clear pending bit in value */
 value &= ~GPTIMER_INT_PENDING;
-} else {
-/* keep pending bit */
+
+/* Read current pending bit from register */
 value |= unit->timers[id].config & GPTIMER_INT_PENDING;
 }


Regards,



Re: [Qemu-devel] [PATCH for-2.10 0/2] etsec: (TYPE_)ETSEC_COMMON macro cleanup

2017-04-03 Thread Fabien Chouteau
On 02/04/2017 14:42, Philippe Mathieu-Daudé wrote:
> On 03/31/2017 04:27 PM, Eduardo Habkost wrote:
>> When working on other things, I got confused by the etsec code,
>> that didn't use the TYPE_ETSEC_COMMON macro in its type
>> declaration, and had no subclasses despite being named
>> ETSEC_COMMON.
>>
>> This is a very simple cleanup to remove the _COMMON suffix from
>> the macros, and to use the TYPE_ETSEC macro when registering and
>> creating devices instead of hardcoding the "eTSEC" name.
>>
>> Cc: Alexander Graf <ag...@suse.de>
>> Cc: Scott Wood <scottw...@freescale.com>
>> Cc: Jason Wang <jasow...@redhat.com>
>> Cc: Fabien Chouteau <chout...@adacore.com>
>> Cc: qemu-...@nongnu.org
>>
>> Eduardo Habkost (2):
>>   etsec: Rename (TYPE_)ETSEC_COMMON to (TYPE_)ETSEC
>>   etsec: Use TYPE_ETSEC macro when registering/creating device
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> 

Reviewed-by: Fabien Chouteau <chout...@adacore.com>


Thanks Eduardo,





[Qemu-devel] QEMU and/or GDB position opening at AdaCore

2016-08-31 Thread Fabien Chouteau
Hello QEMU folks,

AdaCore [1] is opening a QEMU and/or GDB engineer position. You guessed
it, we are looking for someone familiar with low-level programming,
assembly, CPU architectures, etc. On the QEMU side we work on the ARM,
PPC, SPARC and x86 architectures in "full" system emulation only.  Prior
experience with debugger or compiler development is a plus. The location
would be Paris (France) (or maybe New-York (USA)).

I don't have the complete job description yet, but please contact me if
you are interested.

Regards,

[1] http://www.adacore.com/



Re: [Qemu-devel] ARM Cortex-M issues

2016-08-30 Thread Fabien Chouteau
On 08/29/2016 09:19 PM, Liviu Ionescu wrote:
> 
>> On 29 Aug 2016, at 20:59, Bill Paul  wrote:
>>
>> I recently started tinkering with ChibiOS as part of a small personal 
>> project ...
> 
> I did most of the development for the µOS++/CMSIS++ 
> (http://micro-os-plus.github.io) on STM32F4DISCOVERY board, emulated by GNU 
> ARM Eclipse QEMU, which implements even animated LEDs on a graphical image of 
> the board.
> 
> FreeRTOS also works properly on the emulator, both the M0 and M3 ports.
> 
> As for Cortex-M implementation, there are many improvements in the GNU ARM 
> Eclipse QEMU fork (http://gnuarmeclipse.github.io/qemu/), including an 
> Eclipse debug plug-in to start it; it may be worth giving it a try for 
> ChibiOS too. 
> 

There's also the fork from Pebble (the smartwatch): 
https://github.com/pebble/qemu
They seem to have a pretty good Cortex-M support and even I2C, SPI, GPIO...




Re: [Qemu-devel] [PATCH] hw/sparc/leon3: Don't call get_image_size() on a NULL pointer

2016-08-05 Thread Fabien Chouteau
On 08/05/2016 12:03 PM, Peter Maydell wrote:
> get_image_size() doesn't handle being passed a NULL pointer, so
> avoid doing that. Spotted by the clang ub sanitizer (which notices
> the attempt to pass NULL to open()).
> 

Looks good.

Thanks Peter!

> Signed-off-by: Peter Maydell 
> ---
>  hw/sparc/leon3.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index dbae41f..6e16478 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -171,7 +171,11 @@ static void leon3_generic_hw_init(MachineState *machine)
>  }
>  filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>  
> -bios_size = get_image_size(filename);
> +if (filename) {
> +bios_size = get_image_size(filename);
> +} else {
> +bios_size = -1;
> +}
>  
>  if (bios_size > prom_size) {
>  fprintf(stderr, "qemu: could not load prom '%s': file too big\n",
> 




Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

2015-12-07 Thread Fabien Chouteau
On 12/07/2015 02:07 PM, "Züpke, Alexander" wrote:
> Hi Fabien,
> 
>> On December 7, 2015 at 1:56 PM Fabien Chouteau <chout...@adacore.com> wrote:
>> On 12/07/2015 01:41 PM, "Züpke, Alexander" wrote:
>>>
>>> The check for the CASA feature on SPARC v8 is already in the existng code,
>>> just 3 lines above the second hunk in the patch:
>>>
>>
>> Yes but you add a special case that allow casa from user space, this
>> special case is only available on LEON3 so that should be taken into
>> account in the priv_insn check.
>>
>> Regards,
>>
> 
> No, other v8 SPARCs do not have CASA at all, only LEON3 and 4 have CASA
> backported from v9. But when CASA is available on LEON, it's available
> in both user and supervisor mode. The only restriction for user mode
> is that one must use ASI 0xa.
> 
> 

You're right, and ASI 0xA is also user data in SPARKv9.

Patch looks good then :)

Thanks,




Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

2015-12-07 Thread Fabien Chouteau
Hello Alex,

Thanks for your patch! I have a couple of comments. 

On 12/04/2015 04:01 PM, Alex Zuepke wrote:
> LEON3 allows the CASA instruction to be used from user space
> if the ASI is set to 0xa (user data).
> 
> Signed-off-by: Alex Zuepke 
> ---
>  target-sparc/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 41a3319..63440dd 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, 
> unsigned int insn)
>  if (IS_IMM) {
>  goto illegal_insn;
>  }
> -if (!supervisor(dc)) {
> +/* LEON3 allows CASA from user space with ASI 0xa */

Since this is a LEON3 specific feature, when you check the ASI you
should also check CPU_FEATURE_CASA.

> +if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) 
> {
>  goto priv_insn;
>  }

That would give you something like this:

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 41a3319..b93faea 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2463,6 +2463,9 @@ static void gen_faligndata(TCGv dst, TCGv gsr, TCGv s1, 
TCGv s2)
 }
 #endif

+#define IU_FEATURE(dc, FEATURE)\
+(((dc)->def->features & CPU_FEATURE_ ## FEATURE))
+
 #define CHECK_IU_FEATURE(dc, FEATURE)  \
 if (!((dc)->def->features & CPU_FEATURE_ ## FEATURE))  \
 goto illegal_insn;
@@ -5097,7 +5100,11 @@ static void disas_sparc_insn(DisasContext * dc, unsigned 
int insn)
 if (IS_IMM) {
 goto illegal_insn;
 }
-if (!supervisor(dc)) {
+
+/* LEON3 allows CASA from user space with ASI 0xa */
+if (!((IU_FEATURE(dc, CASA) &&
+   (GET_FIELD(insn, 19, 26) == 0xa))
+  || supervisor(dc))) {
 goto priv_insn;
 }
 #endif


to be tested of course ;)

Regards,



Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

2015-12-07 Thread Fabien Chouteau
On 12/07/2015 01:41 PM, "Züpke, Alexander" wrote:
> 
> The check for the CASA feature on SPARC v8 is already in the existng code,
> just 3 lines above the second hunk in the patch:
> 

Yes but you add a special case that allow casa from user space, this
special case is only available on LEON3 so that should be taken into
account in the priv_insn check.

Regards,




Re: [Qemu-devel] [PATCH v2] thread-win32: fix GetThreadContext() permanently fails

2015-07-06 Thread Fabien Chouteau
On 07/02/2015 09:09 PM, Zavadovsky Yan wrote:
 I tested this patch on my 4-cores cpu.
 Debug and release builds both.
 Win32 and Win64 binaries both. (I used old Fedora 17-18 with SJLJ mingw-w64 
 to crossbuild for Win64.)
 With default Qemu BIOS and with myself-builded OVMF(also debug and release) 
 from EDK2.
 
 Also I did some synthetic tests with sample from this article:
 http://blogs.msdn.com/b/oldnewthing/archive/2015/02/05/10591215.aspx
 modified as I described here:
 http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg06894.html
 

That's good for me.

Thanks Yan,




Re: [Qemu-devel] [PATCH v2] thread-win32: fix GetThreadContext() permanently fails

2015-07-02 Thread Fabien Chouteau
On 07/01/2015 08:00 PM, Stefan Weil wrote:
 Am 01.07.2015 um 18:49 schrieb Paolo Bonzini:

 On 01/07/2015 17:48, Zavadovsky Yan wrote:
 Ping.
 Stefan, are you merging this?

 Paolo
 
 I can do so, but as the current code seems to fix the problems
 with multi-processor systems, too (even if it is unclear why),
 it does not look urgent.
 
 Fabien, you suggested extensive tests. Do you think that
 patch v2 is fine, or are you still waiting for test results?
 

The patch looks good. I won't be able to do heavy testing anytime soon,
if Yan tells us that the new code was tested I will take his word for it.

Thanks,




Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails

2015-06-24 Thread Fabien Chouteau
On 06/23/2015 07:07 PM, Stefan Weil wrote:
 Am 23.06.2015 um 12:46 schrieb Paolo Bonzini:
 On 23/06/2015 12:30, Peter Maydell wrote:
 On 23 June 2015 at 10:55, Ян Завадовский zavadovsky@gmail.com wrote:
 On Tue, Jun 23, 2015 at 9:02 AM, Stefan Weil s...@weilnetz.de wrote:
 We should add an URL to reliable documentation which supports that
 claim.
 Unfortunately, MSDN says only SuspendThread suspends the thread. It's
 designed for debuggers. Don't use in applications.:
 https://msdn.microsoft.com/en-us/library/windows/desktop/ms686345(v=vs.85).aspx
 And nothing more useful.
 So when I found this piece of code with Suspend/Resume and failed 
 GetContext
 I did some googling.
 And found this article:
 http://blogs.msdn.com/b/oldnewthing/archive/2015/02/05/10591215.aspx
 Personally I am happy to treat a Raymond Chen blog post as reliable
 documentation...
 Me too. :)
 
 +1
 
 Fabien, I wonder why nobody noticed that the current
 code did not do what it was written for. As far as I see
 the threads were created with the wrong options, so
 GetThreadContext always failed and therefore was only
 executed once, so there was no waiting for thread
 suspension.
 

I'm surprised as well, but we run several hundred thousands of tests
every day (one QEMU instance for each test) and before this fix we had a
few instances freezing for no reason. We identified a possible race
condition on SMP host and the bug disappeared after this fix.

Even if the call was erroneous, adding a call to GetThreadContext
probably gave more time or forced the suspend request to be effective,
it's the only explanation I have right now.

But clearly there was a bug, and the call to GetThreadContext fixed it.
I found other pieces of code that uses this technique but calling
GetThreadContext only once (not in a loop like we did), so maybe it's
enough to call it once and the loop is superfluous...

 Removing the code would have given identical results.


Considering we are talking about thread synchronization on Windows and
SMP host, I would not make that assumption :)

 Is that in an indicator that the SuspendThread is not
 needed at all, as it was discussed in the other e-mails
 here?

If we completely change the thread synchronization on Windows, maybe
SuspendeThread is not needed anymore, but with the current scheme (at
least what I know of it), I don't see how we can remove it.

As I said before we must be very careful with this piece of code.

Regards,



Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails

2015-06-23 Thread Fabien Chouteau
On 06/23/2015 08:02 AM, Stefan Weil wrote:
 Am 22.06.2015 um 23:54 schrieb Zavadovsky Yan:
 Calling SuspendThread() is not enough to suspend Win32 thread.
 We need to call GetThreadContext() after SuspendThread()
 to make sure that OS have really suspended target thread.
 But GetThreadContext() needs for THREAD_GET_CONTEXT
 access right on thread object.

 This patch adds THREAD_GET_CONTEXT to OpenThread() arguments
 and change 'while(GetThreadContext() == SUCCESS)' to
 'while(GetThreadContext() == FAILED)'.
 So this 'while' loop will stop only after successful grabbing
 of thread context(i.e. when thread is really suspended).
 Not after the one failed GetThreadContext() call.

 Signed-off-by: Zavadovsky Yan zavadovsky@gmail.com
 ---
   cpus.c   | 2 +-
   util/qemu-thread-win32.c | 4 ++--
   2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/cpus.c b/cpus.c
 index b85fb5f..83d5eb5 100644
 --- a/cpus.c
 +++ b/cpus.c
 @@ -1097,7 +1097,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
* suspended until we can get the context.
*/
   tcgContext.ContextFlags = CONTEXT_CONTROL;
 -while (GetThreadContext(cpu-hThread, tcgContext) != 0) {
 +while (GetThreadContext(cpu-hThread, tcgContext) == 0) {
   continue;

This looks like a reasonable change, right now I don't understand why I
did it the other way...

   }
   diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
 index 406b52f..823eca1 100644
 --- a/util/qemu-thread-win32.c
 +++ b/util/qemu-thread-win32.c
 @@ -406,8 +406,8 @@ HANDLE qemu_thread_get_handle(QemuThread *thread)
 EnterCriticalSection(data-cs);
   if (!data-exited) {
 -handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME, FALSE,
 -thread-tid);
 +handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME | 
 THREAD_GET_CONTEXT,
 +FALSE, thread-tid);
   } else {
   handle = NULL;
   }
 
 
 I added the contributers of the original code to the cc list.
 
 The modifications look reasonable - if GetThreadContext is needed at all.
 We should add an URL to reliable documentation which supports that
 claim.


The reason we need this call is on multi-processor host, when the TCG
thread and the IO-loop thread don't run on the same CPU.

So in this situation the function SuspendThread can return even before
the thread (running on another CPU) is effectively suspended.

Unfortunately this is not really documented by Microsoft an we found
that information somewhere on Internet (if you want I can search the
source again but there's nothing official) after countless hours of
debugging a very nasty race condition caused by this undocumented
behavior.

Maybe this is not explicit enough and the comments need to be updated.


 Is it a good idea to run a busy waiting loop? Or would a Sleep(0) in
 the loop be better (it allows other threads to run, maybe it helps
 them to suspend, too).


Maybe we can, but the while will only loop when threads are running on
different CPU, so the other thread is already running and calling sleep
will not help I think.

I hope this is clear, as I said we spent a huge amount of time debugging
this about a year and a half ago. The bug would append once every
several thousands tests. QEMU thread code is very sensitive on Windows
so we should be careful.

Yan, if you didn't already, I recommend you extensively test this
modification. By extensively, I mean running QEMU several thousands of
time on an SMP host (with many CPUs like 8 or 16 if possible).

Regards,



[Qemu-devel] [PATCH] [POWERPC] display cpu id dump state

2015-02-25 Thread Fabien Chouteau
From: Tristan Gingold ging...@adacore.com


Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-ppc/translate.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 88c18e3..2a78e99 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11214,8 +11214,9 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
 int i;
 
 cpu_fprintf(f, NIP  TARGET_FMT_lxLR  TARGET_FMT_lx  CTR 
-TARGET_FMT_lx  XER  TARGET_FMT_lx \n,
-env-nip, env-lr, env-ctr, cpu_read_xer(env));
+TARGET_FMT_lx  XER  TARGET_FMT_lx  CPU#%d\n,
+env-nip, env-lr, env-ctr, cpu_read_xer(env),
+cs-cpu_index);
 cpu_fprintf(f, MSR  TARGET_FMT_lx  HID0  TARGET_FMT_lx   HF 
 TARGET_FMT_lx  idx %d\n, env-msr, env-spr[SPR_HID0],
 env-hflags, env-mmu_idx);
-- 
1.7.9.5




[Qemu-devel] [PATCH] Openpic: check that cpu id is within the number of cpus

2015-02-25 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 hw/intc/openpic.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 7d1f3b9..305377d 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -1013,7 +1013,7 @@ static void openpic_cpu_write_internal(void *opaque, 
hwaddr addr,
 DPRINTF(%s: cpu %d addr %# HWADDR_PRIx  = 0x%08x\n, __func__, idx,
 addr, val);
 
-if (idx  0) {
+if (idx  0 || idx = opp-nb_cpus) {
 return;
 }
 
@@ -1152,7 +1152,7 @@ static uint32_t openpic_cpu_read_internal(void *opaque, 
hwaddr addr,
 DPRINTF(%s: cpu %d addr %# HWADDR_PRIx \n, __func__, idx, addr);
 retval = 0x;
 
-if (idx  0) {
+if (idx  0 || idx = opp-nb_cpus) {
 return retval;
 }
 
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH 3/8] leon3: Replace unchecked qdev_init() by qdev_init_nofail()

2015-02-12 Thread Fabien Chouteau
On 02/04/2015 06:33 PM, Markus Armbruster wrote:
 grlib_irqmp_create(), grlib_gptimer_create() and
 grlib_apbuart_create() are helpers to create and realize GRLIB
 devices.  Their only caller leon3_generic_hw_init() doesn't check for
 failure.  Only the first can actually fail, and only when the caller
 fails to set up a pointer property, which is a programming error.
 
 Replace qdev_init() by qdev_init_nofail().
 
 Cc: Fabien Chouteau chout...@adacore.com
 Signed-off-by: Markus Armbruster arm...@redhat.com

Reviewed-by: Fabien Chouteau chout...@adacore.com

Thanks Markus,



[Qemu-devel] [PATCH] target-ppc: Fix breakpoint registers for e300

2014-11-06 Thread Fabien Chouteau
In the previous patch, the registers were added to init_proc_G2LE
instead of init_proc_e300.

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-ppc/translate_init.c |   52 +--
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 20d58c0..1fece7b 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4374,32 +4374,6 @@ static void init_proc_G2LE (CPUPPCState *env)
  SPR_NOACCESS, SPR_NOACCESS,
  spr_read_generic, spr_write_generic,
  0x);
-/* Breakpoints */
-/* XXX : not implemented */
-spr_register(env, SPR_DABR, DABR,
- SPR_NOACCESS, SPR_NOACCESS,
- spr_read_generic, spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_DABR2, DABR2,
- SPR_NOACCESS, SPR_NOACCESS,
- spr_read_generic, spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_IABR2, IABR2,
- SPR_NOACCESS, SPR_NOACCESS,
- spr_read_generic, spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_IBCR, IBCR,
- SPR_NOACCESS, SPR_NOACCESS,
- spr_read_generic, spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_DBCR, DBCR,
- SPR_NOACCESS, SPR_NOACCESS,
- spr_read_generic, spr_write_generic,
- 0x);
 
 /* Memory management */
 gen_low_BATs(env);
@@ -4628,6 +4602,32 @@ static void init_proc_e300 (CPUPPCState *env)
  SPR_NOACCESS, SPR_NOACCESS,
  spr_read_generic, spr_write_generic,
  0x);
+/* Breakpoints */
+/* XXX : not implemented */
+spr_register(env, SPR_DABR, DABR,
+ SPR_NOACCESS, SPR_NOACCESS,
+ spr_read_generic, spr_write_generic,
+ 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_DABR2, DABR2,
+ SPR_NOACCESS, SPR_NOACCESS,
+ spr_read_generic, spr_write_generic,
+ 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_IABR2, IABR2,
+ SPR_NOACCESS, SPR_NOACCESS,
+ spr_read_generic, spr_write_generic,
+ 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_IBCR, IBCR,
+ SPR_NOACCESS, SPR_NOACCESS,
+ spr_read_generic, spr_write_generic,
+ 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_DBCR, DBCR,
+ SPR_NOACCESS, SPR_NOACCESS,
+ spr_read_generic, spr_write_generic,
+ 0x);
 /* Memory management */
 gen_low_BATs(env);
 gen_high_BATs(env);
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH] target-ppc: Fix breakpoint registers for e300

2014-11-06 Thread Fabien Chouteau
On 11/06/2014 06:18 PM, Alexander Graf wrote:
 
 
 On 06.11.14 17:23, Fabien Chouteau wrote:
 In the previous patch, the registers were added to init_proc_G2LE
 instead of init_proc_e300.

 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 Thanks, applied to ppc-next (for 2.2).
 

Thank you,




Re: [Qemu-devel] Better Cortex-M support?

2014-11-04 Thread Fabien Chouteau
On 10/28/2014 11:43 AM, Liviu Ionescu wrote:
 Hi! 
 
 I'm currently maintaining the GNU ARM Eclipse plug-ins 
 (http://gnuarmeclipse.livius.net/blog/), and I'm considering, for the 
 mid-term future, adding a new debugging plug-in to run certain tests under un 
 emulator, and the first choice was QEMU.
 
 Do you know if there are any plans to improve the Cortex-M support? As it is 
 now, for my needs, I would consider it barely usable.
 

I think Francis Alistair [1] is working on STM32 support, search for Netduino 
patches.

We also have partial support for STM32F4 on our repository [2], look inside the 
qemu-stable-2.0.0 branch.

Regards,

[1] alistai...@gmail.com
[2] 
https://forge.open-do.org/plugins/scmgit/cgi-bin/gitweb.cgi?p=couverture-qemu/couverture-qemu.git;a=summary




Re: [Qemu-devel] [PATCH V2] LEON3: Add emulation of AMBA plugplay

2014-10-10 Thread Fabien Chouteau
On 10/09/2014 05:32 PM, Jiri Gaisler wrote:
 
 I am a bit against the merge of AHB and APB initialization into
 the same function. A grlib system can have any number of AHB and APB
 buses, so there really should be a separate init routine per bus
 as in the original patch.
 

I understand your concerns, for the moment the system definition of
leon3 is very static, and I don't see that we will need more flexibility
in the near future. In that case I think it's better to not add
unnecessary complexity in the code.




Re: [Qemu-devel] [PATCH][SPARC] LEON3: Add emulation of AMBA plugplay

2014-10-09 Thread Fabien Chouteau
On 10/08/2014 05:38 PM, Andreas Färber wrote:
 Hi,
 

Hi Andreas,

 Am 08.10.2014 um 16:19 schrieb Fabien Chouteau:
 From: Jiri Gaisler j...@gaisler.se

 +
 +#define TYPE_GRLIB_APB_PNP grlib,apbpnp
 
 If you move the two TYPE_* constants to grlib.h, you can reuse them.
 

Will do.

 +#define GRLIB_APB_PNP(obj) \
 +OBJECT_CHECK(APBPNP, (obj), TYPE_GRLIB_APB_PNP)
 +
 +typedef struct APBPNP {
 +SysBusDevice parent_obj;
 +MemoryRegion iomem;
 +} APBPNP;
 +
 +static uint64_t grlib_apbpnp_read(void *opaque, hwaddr addr,
 +   unsigned size)
 
 Indentation is off by one for all read/write functions.
 

Are you sure? The indentation is 4 spaces right? (checkpatch.pl didn't
raise any error).

 +static int grlib_apbpnp_init(SysBusDevice *dev)
 +{
 +APBPNP *pnp = GRLIB_APB_PNP(dev);
 +
 +memory_region_init_io(pnp-iomem, OBJECT(pnp), grlib_apbpnp_ops, pnp,
 +  apbpnp, APBPNP_REG_SIZE);
 +
 +sysbus_init_mmio(dev, pnp-iomem);

 APBPNP_REG_SIZE seems constant, so you could move both lines into an
 instance_init function.


Will do. I don't need a .class_init then.

 +
 +k-init = grlib_apbpnp_init;
 +}
 +
 +static const TypeInfo grlib_apbpnp_info = {
 +.name  = TYPE_GRLIB_APB_PNP,
 +.parent= TYPE_SYS_BUS_DEVICE,
 +.instance_size = sizeof(APBPNP),
 +.class_init= grlib_apbpnp_class_init,
 +};
 +
 +static void grlib_apbpnp_register_types(void)
 +{
 +type_register_static(grlib_apbpnp_info);
 +}
 +
 +type_init(grlib_apbpnp_register_types)
 
 Please either split into two .c files here, ...
 

 
 ... or if unavoidable use just one type_init and registration function.
 +

I will create one type init for both memory regions.

 +static inline
 +DeviceState *grlib_ahbpnp_create(hwaddr  base)
 +{
 +DeviceState *dev;
 +
 +dev = qdev_create(NULL, grlib,ahbpnp);
 +
 +if (qdev_init(dev)) {
 +return NULL;
 +}
 +
 +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 +
 +return dev;
 +}
 +
  #endif /* ! _GRLIB_H_ */
 
 Are these functions really needed? Can't you just inline them?
 Also note that the return value is never actually checked.


This is what we do for all GRLIB devices, I think it makes a cleaner
machine init.

Thanks for the review.



Re: [Qemu-devel] [PATCH][SPARC] LEON3: Add emulation of AMBA plugplay

2014-10-09 Thread Fabien Chouteau
On 10/08/2014 09:43 PM, Jiri Gaisler wrote:
 On 10/08/2014 05:38 PM, Andreas Färber wrote:
 Hi,

 Am 08.10.2014 um 16:19 schrieb Fabien Chouteau:
 From: Jiri Gaisler j...@gaisler.se

 AMBA plugplay is used by kernels to probe available devices (Timers,
 UART, etc...). This is a static declaration of devices implemented in
 QEMU. In the future, a more advanced version could compute those
 information directly from the device tree.

 Interesting. There's quite some magic numbers in the read functions; I
 wonder if you could read them via QOM if you actually give the devices a
 canonical path or search by type? You may want to peek at ACPI code.
 
 
 The plugplay area is similar in function to the PCI configuration
 space, indicating vendor/device ID's, address range, interrupt number
 etc. of on-chip IP cores. The 'magic' numbers could be generated by
 generic functions taking these parameters as inputs. This would
 certainly make the code more readable, and easily extended in the
 future. Would such a solution be acceptable?
 
 

That would be a great improvement, then we could try to plug it with the
QOM API to generate automatically the data.




[Qemu-devel] [PATCH V2] LEON3: Add emulation of AMBA plugplay

2014-10-09 Thread Fabien Chouteau
From: Jiri Gaisler j...@gaisler.se

AMBA plugplay is used by kernels to probe available devices (Timers,
UART, etc...). This is a static declaration of devices implemented in
QEMU. In the future, a more advanced version could compute those
information directly from the device tree.

Signed-off-by: Fabien Chouteau chout...@adacore.com
---

V2:
 - AHB and APB PNP are now grouped in one device
 - Initialisation moved to .instance_init
 - Minor fixes

 hw/sparc/Makefile.objs   |1 +
 hw/sparc/grlib_ambapnp.c |  149 ++
 hw/sparc/leon3.c |3 +
 include/hw/sparc/grlib.h |   22 +++
 4 files changed, 175 insertions(+)
 create mode 100644 hw/sparc/grlib_ambapnp.c

diff --git a/hw/sparc/Makefile.objs b/hw/sparc/Makefile.objs
index c987b5b..e763701 100644
--- a/hw/sparc/Makefile.objs
+++ b/hw/sparc/Makefile.objs
@@ -1 +1,2 @@
 obj-y += sun4m.o leon3.o
+obj-$(CONFIG_GRLIB) += grlib_ambapnp.o
diff --git a/hw/sparc/grlib_ambapnp.c b/hw/sparc/grlib_ambapnp.c
new file mode 100644
index 000..dd53004
--- /dev/null
+++ b/hw/sparc/grlib_ambapnp.c
@@ -0,0 +1,149 @@
+/*
+ * QEMU GRLIB AMBA PlugPlay Emulator
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include hw/sysbus.h
+#include hw/sparc/grlib.h
+
+/* Size of memory mapped registers */
+#define APBPNP_REG_SIZE (4096 - 8)
+#define AHBPNP_REG_SIZE 4096
+
+#define GRLIB_AMBA_PNP(obj) \
+OBJECT_CHECK(AMBAPNP, (obj), TYPE_GRLIB_AMBA_PNP)
+
+typedef struct AMBAPNP {
+SysBusDevice parent_obj;
+MemoryRegion ahb_iomem;
+MemoryRegion apb_iomem;
+} AMBAPNP;
+
+/* APB PNP */
+
+static uint64_t grlib_apbpnp_read(void *opaque, hwaddr addr,
+   unsigned size)
+{
+uint64_t read_data;
+addr = 0xfff;
+
+/* Unit registers */
+switch (addr  0xffc) {
+case 0x00:
+read_data = 0x0400f000; /* Memory controller */
+break;
+case 0x04:
+read_data = 0xfff1;
+break;
+case 0x08:
+read_data = 0x0100c023; /* APBUART */
+break;
+case 0x0C:
+read_data = 0x0010fff1;
+break;
+case 0x10:
+read_data = 0x0100d040; /* IRQMP */
+break;
+case 0x14:
+read_data = 0x0020fff1;
+break;
+case 0x18:
+read_data = 0x01011006; /* GPTIMER */
+break;
+case 0x1C:
+read_data = 0x0030fff1;
+break;
+
+default:
+read_data = 0;
+}
+if (size == 1) {
+read_data = (24 - (addr  3) * 8);
+read_data = 0x0ff;
+}
+return read_data;
+}
+
+static const MemoryRegionOps grlib_apbpnp_ops = {
+.read   = grlib_apbpnp_read,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+/* AHB PNP */
+
+static uint64_t grlib_ahbpnp_read(void *opaque, hwaddr addr,
+   unsigned size)
+{
+addr = 0xffc;
+
+/* Unit registers */
+switch (addr) {
+case 0:
+return 0x01003000;  /* LEON3 */
+case 0x800:
+return 0x0400f000;  /* Memory controller  */
+case 0x810:
+return 0x0003e002;
+case 0x814:
+return 0x2000e002;
+case 0x818:
+return 0x4003c002;
+case 0x820:
+return 0x01006000;  /* APB bridge @ 0x8000 */
+case 0x830:
+return 0x8000fff2;
+
+default:
+return 0;
+}
+}
+
+static const MemoryRegionOps grlib_ahbpnp_ops = {
+.read = grlib_ahbpnp_read,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void grlib_ambapnp_init(Object *obj)
+{
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+AMBAPNP *pnp = GRLIB_AMBA_PNP(obj);
+
+memory_region_init_io(pnp-ahb_iomem, OBJECT(pnp), grlib_ahbpnp_ops, pnp,
+  ahbpnp, AHBPNP_REG_SIZE);
+sysbus_init_mmio(sbd, pnp-ahb_iomem);
+
+memory_region_init_io(pnp-apb_iomem, OBJECT(pnp), grlib_apbpnp_ops, pnp,
+  apbpnp, APBPNP_REG_SIZE

[Qemu-devel] [PATCH][SPARC] LEON3: Add emulation of AMBA plugplay

2014-10-08 Thread Fabien Chouteau
From: Jiri Gaisler j...@gaisler.se

AMBA plugplay is used by kernels to probe available devices (Timers,
UART, etc...). This is a static declaration of devices implemented in
QEMU. In the future, a more advanced version could compute those
information directly from the device tree.

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 hw/sparc/Makefile.objs   |1 +
 hw/sparc/grlib_ambapnp.c |  206 ++
 hw/sparc/leon3.c |6 ++
 include/hw/sparc/grlib.h |   36 
 4 files changed, 249 insertions(+)
 create mode 100644 hw/sparc/grlib_ambapnp.c

diff --git a/hw/sparc/Makefile.objs b/hw/sparc/Makefile.objs
index c987b5b..e763701 100644
--- a/hw/sparc/Makefile.objs
+++ b/hw/sparc/Makefile.objs
@@ -1 +1,2 @@
 obj-y += sun4m.o leon3.o
+obj-$(CONFIG_GRLIB) += grlib_ambapnp.o
diff --git a/hw/sparc/grlib_ambapnp.c b/hw/sparc/grlib_ambapnp.c
new file mode 100644
index 000..dfadd5c
--- /dev/null
+++ b/hw/sparc/grlib_ambapnp.c
@@ -0,0 +1,206 @@
+/*
+ * QEMU GRLIB AMBA PlugPlay Emulator
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include hw/sysbus.h
+
+#define APBPNP_REG_SIZE 4096 /* Size of memory mapped registers */
+
+#define TYPE_GRLIB_APB_PNP grlib,apbpnp
+#define GRLIB_APB_PNP(obj) \
+OBJECT_CHECK(APBPNP, (obj), TYPE_GRLIB_APB_PNP)
+
+typedef struct APBPNP {
+SysBusDevice parent_obj;
+MemoryRegion iomem;
+} APBPNP;
+
+static uint64_t grlib_apbpnp_read(void *opaque, hwaddr addr,
+   unsigned size)
+{
+uint64_t read_data;
+addr = 0xfff;
+
+/* Unit registers */
+switch (addr  0xffc) {
+case 0x00:
+read_data = 0x0400f000; /* Memory controller */
+break;
+case 0x04:
+read_data = 0xfff1;
+break;
+case 0x08:
+read_data = 0x0100c023; /* APBUART */
+break;
+case 0x0C:
+read_data = 0x0010fff1;
+break;
+case 0x10:
+read_data = 0x0100d040; /* IRQMP */
+break;
+case 0x14:
+read_data = 0x0020fff1;
+break;
+case 0x18:
+read_data = 0x01011006; /* GPTIMER */
+break;
+case 0x1C:
+read_data = 0x0030fff1;
+break;
+
+default:
+read_data = 0;
+}
+if (size == 1) {
+read_data = (24 - (addr  3) * 8);
+read_data = 0x0ff;
+}
+return read_data;
+}
+
+static void grlib_apbpnp_write(void *opaque, hwaddr addr,
+uint64_t value, unsigned size)
+{
+}
+
+static const MemoryRegionOps grlib_apbpnp_ops = {
+.write  = grlib_apbpnp_write,
+.read   = grlib_apbpnp_read,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int grlib_apbpnp_init(SysBusDevice *dev)
+{
+APBPNP *pnp = GRLIB_APB_PNP(dev);
+
+memory_region_init_io(pnp-iomem, OBJECT(pnp), grlib_apbpnp_ops, pnp,
+  apbpnp, APBPNP_REG_SIZE);
+
+sysbus_init_mmio(dev, pnp-iomem);
+
+return 0;
+}
+
+static void grlib_apbpnp_class_init(ObjectClass *klass, void *data)
+{
+SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+k-init = grlib_apbpnp_init;
+}
+
+static const TypeInfo grlib_apbpnp_info = {
+.name  = TYPE_GRLIB_APB_PNP,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(APBPNP),
+.class_init= grlib_apbpnp_class_init,
+};
+
+static void grlib_apbpnp_register_types(void)
+{
+type_register_static(grlib_apbpnp_info);
+}
+
+type_init(grlib_apbpnp_register_types)
+
+
+/* AHB PNP */
+
+#define AHBPNP_REG_SIZE (4096 - 8)  /* Size of memory mapped registers */
+
+#define TYPE_GRLIB_AHB_PNP grlib,ahbpnp
+#define GRLIB_AHB_PNP(obj) \
+OBJECT_CHECK(AHBPNP, (obj), TYPE_GRLIB_AHB_PNP)
+
+typedef struct AHBPNP {
+SysBusDevice parent_obj;
+MemoryRegion iomem;
+} AHBPNP;
+
+static uint64_t grlib_ahbpnp_read(void *opaque, hwaddr addr,
+   unsigned size

[Qemu-devel] [PATCH] Fix typo in eTSEC Ethernet controller

2014-04-02 Thread Fabien Chouteau
IRQ are lowered when ievent bit is cleared, so irq_pulse makes no sense
here...

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 hw/net/fsl_etsec/rings.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index e36cfbe..d4a494f 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -159,7 +159,7 @@ static void ievent_set(eTSEC*etsec,
 
 if ((flags  IEVENT_RXB  etsec-regs[IMASK].value  IMASK_RXBEN)
 || (flags  IEVENT_RXF  etsec-regs[IMASK].value  IMASK_RXFEN)) {
-qemu_irq_pulse(etsec-rx_irq);
+qemu_irq_raise(etsec-rx_irq);
 RING_DEBUG(%s Raise Rx IRQ\n, __func__);
 }
 }
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH] Fix typo in eTSEC Ethernet controller

2014-04-02 Thread Fabien Chouteau
On 04/02/2014 04:52 PM, Alexander Graf wrote:
 
 On 02.04.2014, at 16:49, Fabien Chouteau chout...@adacore.com wrote:
 
 IRQ are lowered when ievent bit is cleared, so irq_pulse makes no sense
 here...

 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 Thanks, applied to ppc-next.

Thanks Alex,

 Given that the wiring with boards is still missing I don't think this is a 
 critical 2.0 fix.
 

That's right.

Regards,




Re: [Qemu-devel] [PATCH] target-ppc: improve info registers by printing SPRs

2014-03-21 Thread Fabien Chouteau
On 03/19/2014 03:17 PM, Alexey Kardashevskiy wrote:
 This adds printing of all SPR registers registered for a CPU.
 
 This removes SPR_ prefix from SPR name to reduce the output.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 

Very useful patch Alexey,

I have few comments:

 +for (i = 0, j = 0; i  ARRAY_SIZE(env-spr_cb); i++) {
 +ppc_spr_t *spr = env-spr_cb[i];
 +
 +if (!spr-name) {
 +continue;
 +}
 +cpu_fprintf(f, %-6s  TARGET_FMT_lx, spr-name, env-spr[i]);
 +j++;
 +if (!(j % 4)) {
 +cpu_fprintf(f, \n);
 +} else {
 +cpu_fprintf(f,  );
 +}
  }

When the number of register is not a multiple of 4 a \n is missing.

e.g.

XER LR  CTR DECR   
SRR0   0010c4d4 SRR1   1030 PID0001 DECAR  000a7d8c
CSRR0   CSRR1   DEAR   01c7201c ESR
IVPR    USPRG0  USPRG4  USPRG5 
USPRG6  USPRG7  TBL TBU
SPRG0  02069030 SPRG1   SPRG2  002724bc SPRG3  80804080
SPRG4   SPRG5   SPRG6   SPRG7  
TBL TBU PIR PVR80210030
DBSR    DBCR0   DBCR1   DBCR2  
IAC1    IAC2    DAC1    DAC2   
TSR TCR05c16000 IVOR0  0100 IVOR1  0200
IVOR2  0300 IVOR3  0400 IVOR4  0500 IVOR5  0600
IVOR6  0700 IVOR7  0800 IVOR8  0900 IVOR9  0a00
IVOR10 0b00 IVOR11 0c00 IVOR12 0d00 IVOR13 0e00
IVOR14 0f00 IVOR15 1000 SPEFSCR  BBEAR  
BBTAR   L1CFG0 3820 NPIDR   IVOR32 1100
IVOR33 1200 IVOR34 1300 IVOR35 1400 MCSRR0 
MCSRR1  MCSR    MCAR    MAS0   00010002
MAS1   80001000 MAS2   01c72004 MAS3   01c72015 MAS4   
MAS6   00010001 PID1    PID2    TLB0CFG 04110200
TLB1CFG 101cc010 EPR MAS7    HID0   80804080
HID1   00023000 L1CSR0 0001 L1CSR1 0001 MMUCSR0 
BUCSR  0201 MMUCFG  SVR (qemu) 


 +#if !defined(CONFIG_USER_ONLY)
  
  #if defined(TARGET_PPC64)
  if (env-flags  POWERPC_FLAG_CFAR) {
 @@ -11233,25 +11201,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
 fprintf_function cpu_fprintf,
  case POWERPC_MMU_2_06a:
  case POWERPC_MMU_2_06d:
  #endif
 -cpu_fprintf(f,  SDR1  TARGET_FMT_lxDAR  TARGET_FMT_lx
 - DSISR  TARGET_FMT_lx \n, env-spr[SPR_SDR1],
 -env-spr[SPR_DAR], env-spr[SPR_DSISR]);
  break;
  case POWERPC_MMU_BOOKE206:
 -cpu_fprintf(f,  MAS0  TARGET_FMT_lx   MAS1  TARGET_FMT_lx
 -  MAS2  TARGET_FMT_lxMAS3  TARGET_FMT_lx 
 \n,
 -env-spr[SPR_BOOKE_MAS0], env-spr[SPR_BOOKE_MAS1],
 -env-spr[SPR_BOOKE_MAS2], env-spr[SPR_BOOKE_MAS3]);
 -
 -cpu_fprintf(f,  MAS4  TARGET_FMT_lx   MAS6  TARGET_FMT_lx
 -  MAS7  TARGET_FMT_lx PID  TARGET_FMT_lx 
 \n,
 -env-spr[SPR_BOOKE_MAS4], env-spr[SPR_BOOKE_MAS6],
 -env-spr[SPR_BOOKE_MAS7], env-spr[SPR_BOOKE_PID]);
 -
 -cpu_fprintf(f, MMUCFG  TARGET_FMT_lx  TLB0CFG  TARGET_FMT_lx
 -TLB1CFG  TARGET_FMT_lx \n,
 -env-spr[SPR_MMUCFG], env-spr[SPR_BOOKE_TLB0CFG],
 -env-spr[SPR_BOOKE_TLB1CFG]);
  break;

If you remove those lines, the switch (env-mmu_model) is empty. You
should remove it entirely then.

Regards,



Re: [Qemu-devel] propose a new idea for GSOC 2014

2014-03-21 Thread Fabien Chouteau
On 03/19/2014 11:51 AM, Stefan Hajnoczi wrote:
 On Tue, Mar 18, 2014 at 9:08 PM, Daniel Smith danielsmith9...@gmail.com 
 wrote:
 I would like to propose a new idea for GSOC 2014 that I want to implement
 for QEMU. Since QEMU are widely used for binary analysis, dynamic binary
 code instrumentation and so on.
 Can we provide the framework like Pin (A Dynamic Binary Instrumentation
 Tool) for those areas in QEMU? Different from that, our framework can
 support both user and kernel level instrumentation.

 Some potential works may include:
 (1) Trace the instructions under a specified system call context (without
 interrupt code);
 (2) Integrate Xed2 for supporting disassembling. Xed2 is a very convenient
 tool and provides bunch of APIs for disassembling.
 (3) Support instrumentation abilities for guest OS event. For example, (1)
 print the value of EAX before a specified instruction gets executed; (2)
 print system call arguments for a certain system call.
 

Interesting idea Daniel,

my company is using QEMU to generate execution traces that are used for
code coverage analysis. We are interested in a more generic binary
analysis integration in QEMU.

Regards,




Re: [Qemu-devel] [PATCH v3] CODING_STYLE: Section about mixed declarations

2014-03-21 Thread Fabien Chouteau
On 03/17/2014 07:26 PM, Eduardo Habkost wrote:
 We had an unwritten rule about declarations having to be at beginning of
 blocks. Make it a written rule.
 

Hello Eduardo,

Is it possible to check this rule in script/checkpatch.pl? (or is it already?)

Regards,



Re: [Qemu-devel] [Qemu-trivial] [PATCH] FSL eTSEC: Fix typo in rx ring

2014-03-17 Thread Fabien Chouteau
On 03/14/2014 06:35 PM, Michael Tokarev wrote:
 14.03.2014 20:51, Fabien Chouteau wrote:
 
  hw/net/fsl_etsec/rings.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
 index 7760272..31e6f58 100644
 --- a/hw/net/fsl_etsec/rings.c
 +++ b/hw/net/fsl_etsec/rings.c
 @@ -592,7 +592,7 @@ void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr)
  
  /* TODO: Broadcast and Multicast */
  
 -if (bd.flags | BD_INTERRUPT) {
 +if (bd.flags  BD_INTERRUPT) {
  /* Set RXFx */
 [..etc..]
 
 Hm.  Has this code _ever_ worked?? :(
 

Yes because the kernel I'm using always set the BD_INTERRUPT flag.



Re: [Qemu-devel] [PULL 073/130] Add Enhanced Three-Speed Ethernet Controller (eTSEC)

2014-03-14 Thread Fabien Chouteau
On 03/14/2014 12:23 PM, Paolo Bonzini wrote:
 Il 07/03/2014 00:33, Alexander Graf ha scritto:
 +if (bd.flags | BD_INTERRUPT) {
 +/* Set RXFx */
 +etsec-regs[RSTAT].value |= 1  (7 - ring_nbr);
 +
 +/* Set IEVENT */
 +ievent_set(etsec, IEVENT_RXF);
 +}
 +
 +} else {
 +if (bd.flags | BD_INTERRUPT) {
 +/* Set IEVENT */
 +ievent_set(etsec, IEVENT_RXB);
 +}
 +}
 
 Coverity flags this bd.flags | BD_INTERRUPT idiom... What did you mean?  
 Can you send a fix to qemu-trivial?
 

That's a very bad mistake indeed, it's supposed to be a bd.flags  
BD_INTERRUPT...




[Qemu-devel] [PATCH] FSL eTSEC: Fix typo in rx ring

2014-03-14 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 hw/net/fsl_etsec/rings.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index 7760272..31e6f58 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -592,7 +592,7 @@ void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr)
 
 /* TODO: Broadcast and Multicast */
 
-if (bd.flags | BD_INTERRUPT) {
+if (bd.flags  BD_INTERRUPT) {
 /* Set RXFx */
 etsec-regs[RSTAT].value |= 1  (7 - ring_nbr);
 
@@ -601,7 +601,7 @@ void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr)
 }
 
 } else {
-if (bd.flags | BD_INTERRUPT) {
+if (bd.flags  BD_INTERRUPT) {
 /* Set IEVENT */
 ievent_set(etsec, IEVENT_RXB);
 }
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH v6] target-sparc: Add and use CPU_FEATURE_CASA

2014-03-12 Thread Fabien Chouteau
Thanks Sebastian, I will try my first pull request ;)




Re: [Qemu-devel] [PATCH v6] target-sparc: Add and use CPU_FEATURE_CASA

2014-03-12 Thread Fabien Chouteau
Thanks Sebastian, I will try my first pull request :)




Re: [Qemu-devel] [PATCH v6] target-sparc: Add and use CPU_FEATURE_CASA

2014-03-12 Thread Fabien Chouteau
On 03/12/2014 11:22 AM, Sebastian Huber wrote:
 Hello Fabien,
 
 On 2014-03-12 11:17, Fabien Chouteau wrote:
 Thanks Sebastian, I will try my first pull request :)
 
 I think Mark already did this
 
 http://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02325.html
 
 ?

Very well then :) Thanks for the udpate.




Re: [Qemu-devel] [PULL 073/130] Add Enhanced Three-Speed Ethernet Controller (eTSEC)

2014-03-12 Thread Fabien Chouteau
On 03/09/2014 09:02 AM, Paolo Bonzini wrote:
 Il 07/03/2014 00:33, Alexander Graf ha scritto:
 From: Fabien Chouteau chout...@adacore.com

 This implementation doesn't include ring priority, TCP/IP Off-Load, QoS.

 Signed-off-by: Fabien Chouteau chout...@adacore.com
 Signed-off-by: Alexander Graf ag...@suse.de
 ---
 
 Is this code dead?  Who uses it?  A quick git grep etsec_create gave no 
 results.



I guess it could be integrated in hw/ppc/mpc8544ds.c



Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA

2014-03-07 Thread Fabien Chouteau
On 03/02/2014 07:11 PM, Andreas Färber wrote:
 Hi Fabien,
 
 Am 14.02.2014 18:27, schrieb Fabien Chouteau:
 On 02/14/2014 04:33 PM, Andreas Färber wrote:
 As for the other one you'll need to sort our who sends a pull if Blue
 doesn't resurface -

 I didn't see any message about this. Does anyone know why he's not around?

 I note that qemu-trivial is not CC'ed here and the
 patch probably isn't anyway. Maybe Fabien can help out with that?


 Andreas I really appreciate your help on this. There's not many patches
 on SPARC/Leon3, but Sebastian's work is very important for this target.
 If you are OK, we can continue with this scheme: Sebastian and I will
 review the patches and you can help us to apply it. What do you think?
 
 Sorry for the late reply. I do not have the capacity to step up as SPARC
 maintainer since it is not related to my work and I am already lagging
 for PReP. My suggestion was rather for you as MAINTAINERS-documented
 Leon3 maintainer to send a pull to Peter once a Leon3 patch got review.
 Just coordinate with Mark and Peter who does what; the key idea is
 someone sending a pull that doesn't break non-sparc targets.
 

Thanks Andreas,

I guess I can be the SPARC MAINTAINERS, my only concerns are the time I
will be able to allocate for this job and the fact that I only know and
use Leon3 in the SPARC architecture. But we can git it a try.



Re: [Qemu-devel] [PATCH v4] target-sparc: Add and use CPU_FEATURE_CASA

2014-02-28 Thread Fabien Chouteau
On 02/28/2014 10:33 AM, Mark Cave-Ayland wrote:
 On 26/02/14 17:25, Peter Maydell wrote:
 
 On 26 February 2014 16:59, Fabien Chouteauchout...@adacore.com  wrote:
 On 02/26/2014 08:56 AM, Sebastian Huber wrote:
 Hello,

 exists there someone who is able to commit this?


 I'm sorry Sebastian, as you probably understood the SPARC maintainer is
 missing which makes commit process more difficult that it used to be...

 Peter, as discussed in a previous mail, do you agree to apply SPARC
 patches that have been reviewed?

 My preference is for one of the people who does care about SPARC
 to take on the job of assembling these patches into pull requests,
 testing them, checking they're not obviously broken or lunatic, etc.
 Mark Cave-Ayland has been doing this recently, so I would prefer
 this patch to go via his tree if he is happy with it and willing to take it.
 
 Hi Sebastian,
 
 My experience with SPARC is purely related to legacy emulation and so I 
 haven't really looked at LEON3 in much detail which makes it tricky for me to 
 review.
 
 Having said that, I'm definitely of the opinion that we should try and apply 
 patches that people find useful where possible. If other people can provide 
 Reviewed-by tags and as long as it doesn't break any of my existing images 
 then I'd be willing to do that.
 

That's great Mark, we will definitely help you with the reviews.




Re: [Qemu-devel] [PATCH v4] target-sparc: Add and use CPU_FEATURE_CASA

2014-02-26 Thread Fabien Chouteau
On 02/26/2014 08:56 AM, Sebastian Huber wrote:
 Hello,
 
 exists there someone who is able to commit this?
 

I'm sorry Sebastian, as you probably understood the SPARC maintainer is
missing which makes commit process more difficult that it used to be...

Peter, as discussed in a previous mail, do you agree to apply SPARC
patches that have been reviewed?

Regards,




Re: [Qemu-devel] [PATCH v2] SPARC: Add and use CPU_FEATURE_CASA

2014-02-14 Thread Fabien Chouteau
On 02/14/2014 09:41 AM, Sebastian Huber wrote:
 On 2014-02-13 16:50, Fabien Chouteau wrote:

 This ASI 0x80 is really defined nowhere in Leon3 not even in the sources :)
 Maybe there's a bug in binutils... Did you try to run this program on a real 
 board?
 
 Yes, I tested it on a NGMP board with a LEON4 processor (documentation is the 
 same for CAS as in LEON3).
 

Alright so we can use the patch as is. 

 The ASI 0x80 is defined in the SPARC V9 manual, Table 12—Address Space 
 Identifiers (ASIs).  Here we have:
 
 0x80, ASI_PRIMARY, Unrestricted access, Primary address space
 
 So should I change it to use User Data Access?
 

No I think supervisor data access closer to the definition in Leon3's doc.




Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA

2014-02-14 Thread Fabien Chouteau
On 02/14/2014 04:33 PM, Andreas Färber wrote:
 As for the other one you'll need to sort our who sends a pull if Blue
 doesn't resurface -

I didn't see any message about this. Does anyone know why he's not around?

 I note that qemu-trivial is not CC'ed here and the
 patch probably isn't anyway. Maybe Fabien can help out with that?
 

Andreas I really appreciate your help on this. There's not many patches
on SPARC/Leon3, but Sebastian's work is very important for this target.
If you are OK, we can continue with this scheme: Sebastian and I will
review the patches and you can help us to apply it. What do you think?

Thanks,




Re: [Qemu-devel] [PATCH] hw/timer/grlib_gptimer: Avoid integer overflow

2014-02-13 Thread Fabien Chouteau
On 02/13/2014 10:16 AM, Sebastian Huber wrote:
 The GPTIMER uses 32-bit registers.  Use a 64-bit operation to get the
 ptimer count, otherwise we end up with a count of 0 for GPTIMER counter
 values of 0x.

Looks good, thanks Sebastian.

Reviewed-by: Fabien Chouteau chout...@adacore.com

 ---
  hw/timer/grlib_gptimer.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
 index d5687f6..343563c 100644
 --- a/hw/timer/grlib_gptimer.c
 +++ b/hw/timer/grlib_gptimer.c
 @@ -106,9 +106,9 @@ static void grlib_gptimer_enable(GPTimer *timer)
  /* ptimer is triggered when the counter reach 0 but GPTimer is triggered 
 at
 underflow. Set count + 1 to simulate the GPTimer behavior. */
  
 -trace_grlib_gptimer_enable(timer-id, timer-counter + 1);
 +trace_grlib_gptimer_enable(timer-id, timer-counter);
  
 -ptimer_set_count(timer-ptimer, timer-counter + 1);
 +ptimer_set_count(timer-ptimer, (uint64_t)timer-counter + 1);
  ptimer_run(timer-ptimer, 1);
  }
  
 




Re: [Qemu-devel] [PATCH v2] SPARC: Add and use CPU_FEATURE_CASA

2014-02-13 Thread Fabien Chouteau
On 02/13/2014 10:52 AM, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.
 
 The CAS synthetic instruction uses an ASI of 0x80.  If TARGET_SPARC64 is
 not defined use a supervisor data load/store for an ASI of 0x80 in
 helper_ld_asi()/helper_st_asi().
 

Hello Sebastian,

If I understand correctly, the difference with V1 is that ASI 0x80. Why
did you chose Supervisor data access against User data access? (I cannot
find documentation about 0x80 ASI)

Thanks,



Re: [Qemu-devel] [PATCH v2] SPARC: Add and use CPU_FEATURE_CASA

2014-02-13 Thread Fabien Chouteau
On 02/13/2014 02:00 PM, Sebastian Huber wrote:
 On 2014-02-13 13:01, Fabien Chouteau wrote:
 On 02/13/2014 10:52 AM, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.

 The CAS synthetic instruction uses an ASI of 0x80.  If TARGET_SPARC64 is
 not defined use a supervisor data load/store for an ASI of 0x80 in
 helper_ld_asi()/helper_st_asi().


 Hello Sebastian,

 If I understand correctly, the difference with V1 is that ASI 0x80. Why
 did you chose Supervisor data access against User data access?
 
 User data access would work also.  I don't have a preference here.
 
 (I cannot
 find documentation about 0x80 ASI)
 
 GCC will generate CAS instructions, e.g.

...

 In the GNU Binutils you find:
 
 opcodes/sparc-opc.c:{ cas,F3(3, 0x3c, 0)|ASI(0x80), F3(~3, ~0x3c, 
 ~0)|ASI(~0x80), [1],2,d, F_ALIAS, 0, v9andleon }, /* casa [rs1]ASI_P,rs2,rd 
 */
 
 This is where the 0x80 comes from.
 

In some leon3 doc I found this:

62.2.7 Compare and Swap instruction (CASA)
LEON3 implements the SPARC V9 Compare and Swap Alternative (CASA) instruction. 
The CASA
is enabled the interger load delay is set to 1 and the NOTAG generic is 0. The 
CASA operates as
described in the SPARC V9 manual. The instruction is privileged but setting ASI 
= 0xA (user data)
will allow it to be used in user mode.

Which confirm privileged instruction. I will ask our GCC expert if they
know where that 0x80 ASI comes from.



Re: [Qemu-devel] [PATCH v2] SPARC: Add and use CPU_FEATURE_CASA

2014-02-13 Thread Fabien Chouteau
On 02/13/2014 03:55 PM, Fabien Chouteau wrote:
 On 02/13/2014 02:00 PM, Sebastian Huber wrote:
 On 2014-02-13 13:01, Fabien Chouteau wrote:
 On 02/13/2014 10:52 AM, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.

 The CAS synthetic instruction uses an ASI of 0x80.  If TARGET_SPARC64 is
 not defined use a supervisor data load/store for an ASI of 0x80 in
 helper_ld_asi()/helper_st_asi().


 Hello Sebastian,

 If I understand correctly, the difference with V1 is that ASI 0x80. Why
 did you chose Supervisor data access against User data access?

 User data access would work also.  I don't have a preference here.

 (I cannot
 find documentation about 0x80 ASI)

 GCC will generate CAS instructions, e.g.
 
 ...
 
 In the GNU Binutils you find:

 opcodes/sparc-opc.c:{ cas,F3(3, 0x3c, 0)|ASI(0x80), F3(~3, ~0x3c, 
 ~0)|ASI(~0x80), [1],2,d, F_ALIAS, 0, v9andleon }, /* casa 
 [rs1]ASI_P,rs2,rd */

 This is where the 0x80 comes from.

 
 In some leon3 doc I found this:
 
 62.2.7 Compare and Swap instruction (CASA)
 LEON3 implements the SPARC V9 Compare and Swap Alternative (CASA) 
 instruction. The CASA
 is enabled the interger load delay is set to 1 and the NOTAG generic is 0. 
 The CASA operates as
 described in the SPARC V9 manual. The instruction is privileged but setting 
 ASI = 0xA (user data)
 will allow it to be used in user mode.
 
 Which confirm privileged instruction. I will ask our GCC expert if they
 know where that 0x80 ASI comes from.
 

This ASI 0x80 is really defined nowhere in Leon3 not even in the sources :)
Maybe there's a bug in binutils... Did you try to run this program on a real 
board?

--- sparc.vhd ---

subtype asi_type is std_logic_vector(4 downto 0);

constant ASI_SYSR: asi_type := 00010; -- 0x02
constant ASI_UINST   : asi_type := 01000; -- 0x08
constant ASI_SINST   : asi_type := 01001; -- 0x09
constant ASI_UDATA   : asi_type := 01010; -- 0x0A
constant ASI_SDATA   : asi_type := 01011; -- 0x0B
constant ASI_ITAG: asi_type := 01100; -- 0x0C
constant ASI_IDATA   : asi_type := 01101; -- 0x0D
constant ASI_DTAG: asi_type := 01110; -- 0x0E
constant ASI_DDATA   : asi_type := 0; -- 0x0F
constant ASI_IFLUSH  : asi_type := 1; -- 0x10
constant ASI_DFLUSH  : asi_type := 10001; -- 0x11

constant ASI_FLUSH_PAGE : std_logic_vector(4 downto 0) := 1;  -- 0x10 
i/dcache flush page
constant ASI_FLUSH_CTX  : std_logic_vector(4 downto 0) := 10011;  -- 0x13 
i/dcache flush ctx

constant ASI_DCTX   : std_logic_vector(4 downto 0) := 10100;  -- 0x14 
dcache ctx
constant ASI_ICTX   : std_logic_vector(4 downto 0) := 10101;  -- 0x15 
icache ctx

constant ASI_MMUFLUSHPROBE  : std_logic_vector(4 downto 0) := 11000;  -- 0x18 
i/dtlb flush/(probe)
constant ASI_MMUREGS: std_logic_vector(4 downto 0) := 11001;  -- 0x19 
mmu regs access
constant ASI_MMU_BP : std_logic_vector(4 downto 0) := 11100;  -- 0x1c 
mmu Bypass 
constant ASI_MMU_DIAG   : std_logic_vector(4 downto 0) := 11101;  -- 0x1d 
mmu diagnostic 
--constant ASI_MMU_DSU: std_logic_vector(4 downto 0) := 1;  -- 
0x1f mmu diagnostic 

constant ASI_MMUSNOOP_DTAG  : std_logic_vector(4 downto 0) := 0;  -- 0x1e 
mmusnoop physical dtag 




Re: [Qemu-devel] [PATCH] sparc/leon3: Initialize stack pointer

2014-02-05 Thread Fabien Chouteau
On 02/03/2014 10:18 AM, Sebastian Huber wrote:
 A lot of real world LEON3 systems are shipped with the GRMON boot
 loader.  This boot loader initializes the stack pointer with the end of
 RAM address.  The application can use this to detect the RAM size of a
 particular board variant.


Looks good, thank you Sebastian.

Reviewed-by: Fabien Chouteau chout...@adacore.com

 
 Signed-off-by: Sebastian Huber sebastian.hu...@embedded-brains.de
 ---
  hw/sparc/leon3.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
 index c583c3d..c16e9e4 100644
 --- a/hw/sparc/leon3.c
 +++ b/hw/sparc/leon3.c
 @@ -45,6 +45,7 @@
  typedef struct ResetData {
  SPARCCPU *cpu;
  uint32_t  entry;/* save kernel entry in case of reset */
 +target_ulong sp;/* initial stack pointer */
  } ResetData;
  
  static void main_cpu_reset(void *opaque)
 @@ -58,6 +59,7 @@ static void main_cpu_reset(void *opaque)
  cpu-halted = 0;
  env-pc = s-entry;
  env-npc= s-entry + 4;
 +env-regbase[6] = s-sp;
  }
  
  void leon3_irq_ack(void *irq_manager, int intno)
 @@ -133,6 +135,7 @@ static void leon3_generic_hw_init(QEMUMachineInitArgs 
 *args)
  /* Reset data */
  reset_info= g_malloc0(sizeof(ResetData));
  reset_info-cpu   = cpu;
 +reset_info-sp= 0x4000 + ram_size;
  qemu_register_reset(main_cpu_reset, reset_info);
  
  /* Allocate IRQ manager */
 




Re: [Qemu-devel] [PATCH] SPARC: Add and use CPU_FEATURE_CASA

2014-02-05 Thread Fabien Chouteau
On 11/26/2013 03:04 PM, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.

The patch looks good. I can't really test it but I assume you did. 

Thank you Sebastian.

Reviewed-by: Fabien Chouteau chout...@adacore.com

 ---
  target-sparc/cpu.c |3 +-
  target-sparc/cpu.h |4 ++-
  target-sparc/helper.h  |4 ++-
  target-sparc/ldst_helper.c |   26 +---
  target-sparc/translate.c   |   47 ---
  5 files changed, 52 insertions(+), 32 deletions(-)
 
 diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
 index e7f878e..5806e59 100644
 --- a/target-sparc/cpu.c
 +++ b/target-sparc/cpu.c
 @@ -458,7 +458,8 @@ static const sparc_def_t sparc_defs[] = {
  .mmu_trcr_mask = 0x,
  .nwindows = 8,
  .features = CPU_DEFAULT_FEATURES | CPU_FEATURE_TA0_SHUTDOWN |
 -CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN,
 +CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL | CPU_FEATURE_POWERDOWN |
 +CPU_FEATURE_CASA,
  },
  #endif
  };
 diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
 index 41194ec..f87d7fb 100644
 --- a/target-sparc/cpu.h
 +++ b/target-sparc/cpu.h
 @@ -271,12 +271,14 @@ typedef struct sparc_def_t {
  #define CPU_FEATURE_ASR17(1  15)
  #define CPU_FEATURE_CACHE_CTRL   (1  16)
  #define CPU_FEATURE_POWERDOWN(1  17)
 +#define CPU_FEATURE_CASA (1  18)
  
  #ifndef TARGET_SPARC64
  #define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP |  \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
CPU_FEATURE_FLUSH | CPU_FEATURE_FSQRT | \
 -  CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD)
 +  CPU_FEATURE_FMUL | CPU_FEATURE_FSMULD | \
 +  CPU_FEATURE_CASA)
  #else
  #define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP |  \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
 diff --git a/target-sparc/helper.h b/target-sparc/helper.h
 index 5e0eea1..9c4fd56 100644
 --- a/target-sparc/helper.h
 +++ b/target-sparc/helper.h
 @@ -22,7 +22,6 @@ DEF_HELPER_1(popc, tl, tl)
  DEF_HELPER_4(ldda_asi, void, env, tl, int, int)
  DEF_HELPER_5(ldf_asi, void, env, tl, int, int, int)
  DEF_HELPER_5(stf_asi, void, env, tl, int, int, int)
 -DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
  DEF_HELPER_5(casx_asi, tl, env, tl, tl, tl, i32)
  DEF_HELPER_2(set_softint, void, env, i64)
  DEF_HELPER_2(clear_softint, void, env, i64)
 @@ -31,6 +30,9 @@ DEF_HELPER_2(tick_set_count, void, ptr, i64)
  DEF_HELPER_1(tick_get_count, i64, ptr)
  DEF_HELPER_2(tick_set_limit, void, ptr, i64)
  #endif
 +#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
 +DEF_HELPER_5(cas_asi, tl, env, tl, tl, tl, i32)
 +#endif
  DEF_HELPER_3(check_align, void, env, tl, i32)
  DEF_HELPER_1(debug, void, env)
  DEF_HELPER_1(save, void, env)
 diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
 index 2936b58..c51b9b0 100644
 --- a/target-sparc/ldst_helper.c
 +++ b/target-sparc/ldst_helper.c
 @@ -2224,33 +2224,35 @@ void helper_stf_asi(CPUSPARCState *env, target_ulong 
 addr, int asi, int size,
  }
  }
  
 -target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
 -target_ulong val1, target_ulong val2, uint32_t 
 asi)
 +target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
 + target_ulong val1, target_ulong val2,
 + uint32_t asi)
  {
  target_ulong ret;
  
 -val2 = 0xUL;
 -ret = helper_ld_asi(env, addr, asi, 4, 0);
 -ret = 0xUL;
 +ret = helper_ld_asi(env, addr, asi, 8, 0);
  if (val2 == ret) {
 -helper_st_asi(env, addr, val1  0xUL, asi, 4);
 +helper_st_asi(env, addr, val1, asi, 8);
  }
  return ret;
  }
 +#endif /* TARGET_SPARC64 */
  
 -target_ulong helper_casx_asi(CPUSPARCState *env, target_ulong addr,
 - target_ulong val1, target_ulong val2,
 - uint32_t asi)
 +#if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
 +target_ulong helper_cas_asi(CPUSPARCState *env, target_ulong addr,
 +target_ulong val1, target_ulong val2, uint32_t 
 asi)
  {
  target_ulong ret;
  
 -ret = helper_ld_asi(env, addr, asi, 8, 0);
 +val2 = 0xUL;
 +ret = helper_ld_asi(env, addr, asi, 4, 0);
 +ret = 0xUL;
  if (val2 == ret) {
 -helper_st_asi(env, addr, val1, asi, 8);
 +helper_st_asi(env, addr, val1  0xUL, asi, 4);
  }
  return ret;
  }
 -#endif /* TARGET_SPARC64 */
 +#endif /* !defined

Re: [Qemu-devel] [Qemu-trivial] [PATCH] sparc/leon3: Initialize stack pointer

2014-02-05 Thread Fabien Chouteau
On 02/05/2014 10:02 AM, Fabien Chouteau wrote:
 On 02/03/2014 10:18 AM, Sebastian Huber wrote:
 A lot of real world LEON3 systems are shipped with the GRMON boot
 loader.  This boot loader initializes the stack pointer with the end of
 RAM address.  The application can use this to detect the RAM size of a
 particular board variant.

 
 Looks good, thank you Sebastian.
 
 Reviewed-by: Fabien Chouteau chout...@adacore.com
 

Sorry Blue you should be in copy. Can you apply this patch please?

Thanks,




Re: [Qemu-devel] [PATCH] SPARC: Add and use CPU_FEATURE_CASA

2014-02-05 Thread Fabien Chouteau
On 02/05/2014 10:21 AM, Fabien Chouteau wrote:
 On 11/26/2013 03:04 PM, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.
 
 The patch looks good. I can't really test it but I assume you did. 
 
 Thank you Sebastian.
 
 Reviewed-by: Fabien Chouteau chout...@adacore.com
 

Sorry Blue you should be in copy. Can you apply this patch please?

Thanks,




Re: [Qemu-devel] [PATCH] SPARC: Add and use CPU_FEATURE_CASA

2014-02-05 Thread Fabien Chouteau
On 11/28/2013 10:55 AM, Sebastian Huber wrote:
 Hello,
 
 On 2013-11-26 15:04, Sebastian Huber wrote:
 The LEON3 processor has support for the CASA instruction which is
 normally only available for SPARC V9 processors.  Binutils 2.24
 and GCC 4.9 will support this instruction for LEON3.  GCC uses it to
 generate C11 atomic operations.
 ---
   target-sparc/cpu.c |3 +-
   target-sparc/cpu.h |4 ++-
   target-sparc/helper.h  |4 ++-
   target-sparc/ldst_helper.c |   26 +---
   target-sparc/translate.c   |   47 
 ---
   5 files changed, 52 insertions(+), 32 deletions(-)
 [...]
 
 this patch doesn't work since the ASI 0x80 used for the synthetic CAS 
 instruction is not implemented in helper_ld_asi() for !TARGET_SPARC64.
 
 I tried to add a
 
 case 0x80: /* Primary */
 {
 switch (size) {
 case 1:
 ret = ldub_raw(addr);
 break;
 case 2:
 ret = lduw_raw(addr);
 break;
 case 4:
 ret = ldl_raw(addr);
 break;
 default:
 case 8:
 ret = ldq_raw(addr);
 break;
 }
 }
 break;
 
 but this results in a Qemu segmentation fault.
 

Hello Sebastian,

I missed this email. It's easier for me to see you message if I'm in
copy, also add Blue Swirl blauwir...@gmail.com in copy for all SPARC
patches.

ASI 0x80 doesn't make sense in SPARC32 where does this value come from?
I guess it's TCGv_i32 r_asi = tcg_const_i32(GET_FIELD(insn, 19, 26));,
right?




Re: [Qemu-devel] [PATCH V2] Add Enhanced Three-Speed Ethernet Controller (eTSEC)

2014-02-05 Thread Fabien Chouteau
On 12/10/2013 06:20 PM, Alexander Graf wrote:
 On 10.12.2013, at 18:15, Fabien Chouteau chout...@adacore.com wrote:
 Unfortunately I won't have time to fix before January.
 

Hello Alex,

I fixed the compiler errors and I will send version 3 in few minutes. 

 That works for me. While at it, please also add a patch to enable -net usage 
 with etsec.
 

-net nic,model=ETSEC,vlan=0 already works.

Regards,




Re: [Qemu-devel] [PATCH V2] Add Enhanced Three-Speed Ethernet Controller (eTSEC)

2013-12-10 Thread Fabien Chouteau
On 12/10/2013 01:48 PM, Alexander Graf wrote:
 
 On 10.12.2013, at 12:59, Alexey Kardashevskiy a...@ozlabs.ru wrote:
 
 On 12/10/2013 02:15 AM, Alexander Graf wrote:

 On 22.07.2013, at 18:00, Fabien Chouteau chout...@adacore.com wrote:

 This implementation doesn't include ring priority, TCP/IP Off-Load, QoS.

 Signed-off-by: Fabien Chouteau chout...@adacore.com

 Thanks, applied to ppc-next. Could you please follow up with a patch that 
 makes this available as -net nic,model=etsec and generate a respective 
 device tree node for it?

 This does not compile:
 
 Ok, removed from ppc-next again. Fabien, please rebase on the current git 
 state :).
 

Thanks Alex, I'm surprised I though this patch was forgotten :)

Unfortunately I won't have time to fix before January.





Re: [Qemu-devel] [PATCH v2] SPARC: Add and use CPU_FEATURE_CASA

2013-12-10 Thread Fabien Chouteau
On 12/10/2013 09:09 AM, Sebastian Huber wrote:
 Hello,
 
 would someone please have a look at this.
 

I'm sorry Sebastian I don't have time to look at it. Maybe in January...




Re: [Qemu-devel] [PATCH] SPARC: Fix LEON3 power down instruction

2013-11-26 Thread Fabien Chouteau
On 11/25/2013 03:22 PM, Sebastian Huber wrote:
 The env-pc is not necessarily up-to-date in the helper function.  Use
 the program counter of the disassembly context instead.
 

Looks good. Thanks Sebastian.

Reviewed-by: Fabien Chouteau chout...@adacore.com

 Signed-off-by: Sebastian Huber sebastian.hu...@embedded-brains.de
 ---
  target-sparc/helper.c|6 +++---
  target-sparc/helper.h|2 +-
  target-sparc/translate.c |3 ++-
  3 files changed, 6 insertions(+), 5 deletions(-)
 
 diff --git a/target-sparc/helper.c b/target-sparc/helper.c
 index e70d1bc..50912ff 100644
 --- a/target-sparc/helper.c
 +++ b/target-sparc/helper.c
 @@ -314,14 +314,14 @@ target_ulong helper_tsubcctv(CPUSPARCState *env, 
 target_ulong src1,
  }
  
  #ifndef TARGET_SPARC64
 -void helper_power_down(CPUSPARCState *env)
 +void helper_power_down(CPUSPARCState *env, uint32_t pc)
  {
  CPUState *cs = CPU(sparc_env_get_cpu(env));
  
  cs-halted = 1;
  env-exception_index = EXCP_HLT;
 -env-pc = env-npc;
 -env-npc = env-pc + 4;
 +env-pc = pc + 4;
 +env-npc = pc + 8;
  cpu_loop_exit(env);
  }
  #endif
 diff --git a/target-sparc/helper.h b/target-sparc/helper.h
 index fc49cd8..c4752c7 100644
 --- a/target-sparc/helper.h
 +++ b/target-sparc/helper.h
 @@ -6,7 +6,7 @@ DEF_HELPER_2(trace_insn, void, env, i32)
  DEF_HELPER_1(rett, void, env)
  DEF_HELPER_2(wrpsr, void, env, tl)
  DEF_HELPER_1(rdpsr, tl, env)
 -DEF_HELPER_1(power_down, void, env)
 +DEF_HELPER_2(power_down, void, env, i32)
  #else
  DEF_HELPER_2(wrpil, void, env, tl)
  DEF_HELPER_2(wrpstate, void, env, tl)
 diff --git a/target-sparc/translate.c b/target-sparc/translate.c
 index 0588d23..d9ee90c 100644
 --- a/target-sparc/translate.c
 +++ b/target-sparc/translate.c
 @@ -3631,7 +3631,8 @@ static void disas_sparc_insn(DisasContext * dc, 
 unsigned int insn)
  if ((rd == 0x13)  (dc-def-features 
   CPU_FEATURE_POWERDOWN)) 
 {
  /* LEON3 power-down */
 -gen_helper_power_down(cpu_env);
 +tcg_gen_movi_i32(cpu_tmp0, dc-pc);
 +gen_helper_power_down(cpu_env, cpu_tmp0);
  }
  break;
  #else
 




Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-07 Thread Fabien Chouteau
On 10/04/2013 07:49 PM, Peter Maydell wrote:
 On 5 October 2013 01:57, Fabien Chouteau chout...@adacore.com wrote:
  @@ -47,7 +48,9 @@
  #include hw/xen/xen.h
  #include hw/i386/apic_internal.h
  #endif
 +#include monitor/monitor_def.h

 +extern const MonitorDef i386_monitor_defs[];
 
 Declare this in cpu-qom.h, rather than having an
 extern declaration in a .c file.
 

I didn't manage to do that.

 
 --- a/target-sparc/cpu-qom.h
 +++ b/target-sparc/cpu-qom.h
 @@ -21,7 +21,6 @@
  #define QEMU_SPARC_CPU_QOM_H

  #include qom/cpu.h
 -#include cpu.h
 
 ...why have you deleted this #include ?
 

I thought I added it myself, that's why I removed it.

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 4/4] Add ARM registers definitions for Monitor commands

2013-10-07 Thread Fabien Chouteau
On 10/04/2013 07:52 PM, Peter Maydell wrote:
 +#include monitor/monitor_def.h
 +
 +const MonitorDef arm_monitor_defs[] = {
 +{ r0, offsetof(CPUARMState, regs[0])  },
 +{ r1, offsetof(CPUARMState, regs[1])  },
 
 These fields are all 32 bits, not target_long,
 so they need to be marked as MD_I32. (If you build an
 aarch64-softmmu target then it will have target_long be
 64 bit but still support all the 32 bit CPUs, so it does
 make a difference.)
 

OK, I'll mark them MD_I32.

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-07 Thread Fabien Chouteau
On 10/07/2013 01:45 PM, Peter Maydell wrote:
 On 7 October 2013 19:11, Fabien Chouteau chout...@adacore.com wrote:
 On 10/04/2013 07:49 PM, Peter Maydell wrote:
 On 5 October 2013 01:57, Fabien Chouteau chout...@adacore.com wrote:
 @@ -47,7 +48,9 @@
  #include hw/xen/xen.h
  #include hw/i386/apic_internal.h
  #endif
 +#include monitor/monitor_def.h

 +extern const MonitorDef i386_monitor_defs[];

 Declare this in cpu-qom.h, rather than having an
 extern declaration in a .c file.


 I didn't manage to do that.
 
 If you just say I couldn't make that work with no details,
 there isn't much I can say beyond try harder :-)
 If you say *why* you couldn't get it to work then I'm more
 likely to either (a) suggest an approach that will work or
 (b) accept that it really does have to be that way...
 

Sorry I sent the email too quickly. It looks like a circular dependency

In file included from /home/chouteau/src/qemu-main/target-arm/cpu.h:294:0,
 from 
/home/chouteau/src/qemu-main/include/monitor/monitor_def.h:4,
 from /home/chouteau/src/qemu-main/target-arm/monitor.c:20:
/home/chouteau/src/qemu-main/target-arm/cpu-qom.h:184:25: error: array type has 
incomplete element type
make[1]: *** [target-arm/monitor.o] Error 1

monitor_def.h - cpu.h - cpu-qom.h -.
  ^  |
  |__/


-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-07 Thread Fabien Chouteau
On 10/07/2013 03:29 PM, Peter Maydell wrote:
 On 7 October 2013 22:06, Fabien Chouteau chout...@adacore.com wrote:
 On 10/07/2013 01:45 PM, Peter Maydell wrote:
 On 7 October 2013 19:11, Fabien Chouteau chout...@adacore.com wrote:
 On 10/04/2013 07:49 PM, Peter Maydell wrote:
 On 5 October 2013 01:57, Fabien Chouteau chout...@adacore.com wrote:
 +extern const MonitorDef i386_monitor_defs[];

 Declare this in cpu-qom.h, rather than having an
 extern declaration in a .c file.
 
 Sorry I sent the email too quickly. It looks like a circular dependency

 In file included from /home/chouteau/src/qemu-main/target-arm/cpu.h:294:0,
  from 
 /home/chouteau/src/qemu-main/include/monitor/monitor_def.h:4,
  from /home/chouteau/src/qemu-main/target-arm/monitor.c:20:
 /home/chouteau/src/qemu-main/target-arm/cpu-qom.h:184:25: error: array type 
 has incomplete element type
 make[1]: *** [target-arm/monitor.o] Error 1
 
 I think you should be able to declare it as
   extern const MonitorDef *i386_monitor_defs;
 
 then you don't need to include monitor_def.h from cpu-qom.h.
 (untested, but the typedef should be sufficient for this)
 

/home/chouteau/src/qemu-main/target-arm/monitor.c:22:19: error: conflicting 
types for ‘arm_monitor_defs’
/home/chouteau/src/qemu-main/target-arm/cpu-qom.h:183:26: note: previous 
declaration of ‘arm_monitor_defs’ was here

cpu-qom.h is indirectly included in target-arm/monitor.c

-- 
Fabien Chouteau



[Qemu-devel] [PATCH 4/4] Add ARM registers definitions for Monitor commands

2013-10-04 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-arm/Makefile.objs |2 +-
 target-arm/cpu.c |7 +++
 target-arm/monitor.c |   40 
 3 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 target-arm/monitor.c

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 6453f5c..6c3ec32 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -1,5 +1,5 @@
 obj-y += arm-semi.o
-obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_SOFTMMU) += machine.o monitor.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += translate.o op_helper.o helper.o cpu.o
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d40f2a7..bb15ee8 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -26,6 +26,9 @@
 #include hw/arm/arm.h
 #include sysemu/sysemu.h
 #include sysemu/kvm.h
+#include monitor/monitor_def.h
+
+extern const MonitorDef arm_monitor_defs[];
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -217,6 +220,10 @@ static void arm_cpu_initfn(Object *obj)
ARRAY_SIZE(cpu-gt_timer_outputs));
 #endif
 
+#if !defined(CONFIG_USER_ONLY)
+cs-monitor_defs = arm_monitor_defs;
+#endif
+
 if (tcg_enabled()  !inited) {
 inited = true;
 arm_translate_init();
diff --git a/target-arm/monitor.c b/target-arm/monitor.c
new file mode 100644
index 000..0d68a14
--- /dev/null
+++ b/target-arm/monitor.c
@@ -0,0 +1,40 @@
+/*
+ * ARM MonitorDef
+ *
+ * Copyright (c) 2013 AdaCore
+ *
+ * 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/.
+ */
+
+#include monitor/monitor_def.h
+
+const MonitorDef arm_monitor_defs[] = {
+{ r0, offsetof(CPUARMState, regs[0])  },
+{ r1, offsetof(CPUARMState, regs[1])  },
+{ r2, offsetof(CPUARMState, regs[2])  },
+{ r3, offsetof(CPUARMState, regs[3])  },
+{ r4, offsetof(CPUARMState, regs[4])  },
+{ r5, offsetof(CPUARMState, regs[5])  },
+{ r6, offsetof(CPUARMState, regs[6])  },
+{ r7, offsetof(CPUARMState, regs[7])  },
+{ r8, offsetof(CPUARMState, regs[8])  },
+{ r9, offsetof(CPUARMState, regs[9])  },
+{ r10,offsetof(CPUARMState, regs[10]) },
+{ r11,offsetof(CPUARMState, regs[11]) },
+{ r12,offsetof(CPUARMState, regs[12]) },
+{ r13|sp, offsetof(CPUARMState, regs[13]) },
+{ r14|lr, offsetof(CPUARMState, regs[14]) },
+{ r15|pc, offsetof(CPUARMState, regs[15]) },
+{ NULL },
+};
-- 
1.7.9.5




[Qemu-devel] [PATCH 1/4] Fix coding style

2013-10-04 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 disas.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/disas.c b/disas.c
index 0203ef2..32407de 100644
--- a/disas.c
+++ b/disas.c
@@ -506,12 +506,13 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 return;
 #endif
 
-for(i = 0; i  nb_insn; i++) {
-   monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
+for (i = 0; i  nb_insn; i++) {
+monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
 count = print_insn(pc, s.info);
-   monitor_printf(mon, \n);
-   if (count  0)
-   break;
+monitor_printf(mon, \n);
+if (count  0) {
+break;
+}
 pc += count;
 }
 }
-- 
1.7.9.5




[Qemu-devel] [PATCH 0/4] Few minor improvements of monitor disas command (v4)

2013-10-04 Thread Fabien Chouteau
Another attempt. I think this is the best I can do to fulfil all comments.

Fabien Chouteau (4):
  Fix coding style
  Improve Monitor disas with symbol lookup
  Refactoring MonitorDef array
  Add ARM registers definitions for Monitor commands

 disas.c   |   19 +-
 include/monitor/monitor_def.h |   18 ++
 include/qemu/typedefs.h   |1 +
 include/qom/cpu.h |3 +
 monitor.c |  396 +
 target-arm/Makefile.objs  |2 +-
 target-arm/cpu.c  |7 +
 target-arm/monitor.c  |   40 +
 target-i386/Makefile.objs |2 +-
 target-i386/cpu-qom.h |1 +
 target-i386/cpu.c |7 +
 target-i386/monitor.c |   62 +++
 target-ppc/Makefile.objs  |2 +-
 target-ppc/monitor.c  |  234 
 target-ppc/translate_init.c   |8 +
 target-sparc/Makefile.objs|2 +-
 target-sparc/cpu-qom.h|1 -
 target-sparc/cpu.c|7 +
 target-sparc/monitor.c|  138 ++
 19 files changed, 552 insertions(+), 398 deletions(-)
 create mode 100644 include/monitor/monitor_def.h
 create mode 100644 target-arm/monitor.c
 create mode 100644 target-i386/monitor.c
 create mode 100644 target-ppc/monitor.c
 create mode 100644 target-sparc/monitor.c

-- 
1.7.9.5




[Qemu-devel] [PATCH 2/4] Improve Monitor disas with symbol lookup

2013-10-04 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 disas.c |   10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index 32407de..c83bf5b 100644
--- a/disas.c
+++ b/disas.c
@@ -507,7 +507,15 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 #endif
 
 for (i = 0; i  nb_insn; i++) {
-monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
+const char *sym = lookup_symbol(pc);
+
+monitor_printf(mon, 0x TARGET_FMT_lx, pc);
+if (sym[0] != '\0') {
+monitor_printf(mon,  %s:  , sym);
+} else {
+monitor_printf(mon, :  );
+}
+
 count = print_insn(pc, s.info);
 monitor_printf(mon, \n);
 if (count  0) {
-- 
1.7.9.5




[Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-04 Thread Fabien Chouteau
Everything has been moved to cpu specific directories (SPARC, PPC,
i386).

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 include/monitor/monitor_def.h |   18 ++
 include/qemu/typedefs.h   |1 +
 include/qom/cpu.h |3 +
 monitor.c |  396 +
 target-i386/Makefile.objs |2 +-
 target-i386/cpu-qom.h |1 +
 target-i386/cpu.c |7 +
 target-i386/monitor.c |   62 +++
 target-ppc/Makefile.objs  |2 +-
 target-ppc/monitor.c  |  234 
 target-ppc/translate_init.c   |8 +
 target-sparc/Makefile.objs|2 +-
 target-sparc/cpu-qom.h|1 -
 target-sparc/cpu.c|7 +
 target-sparc/monitor.c|  138 ++
 15 files changed, 490 insertions(+), 392 deletions(-)
 create mode 100644 include/monitor/monitor_def.h
 create mode 100644 target-i386/monitor.c
 create mode 100644 target-ppc/monitor.c
 create mode 100644 target-sparc/monitor.c

diff --git a/include/monitor/monitor_def.h b/include/monitor/monitor_def.h
new file mode 100644
index 000..7d4e3b6
--- /dev/null
+++ b/include/monitor/monitor_def.h
@@ -0,0 +1,18 @@
+#ifndef _MONITOR_DEF_H_
+#define _MONITOR_DEF_H_
+
+#include cpu.h
+
+CPUArchState *mon_get_cpu(void);
+
+#define MD_TLONG 0
+#define MD_I32   1
+
+typedef struct MonitorDef {
+const char *name;
+int offset;
+target_long (*get_value)(const struct MonitorDef *md, int val);
+int type;
+} MonitorDef;
+
+#endif /* ! _MONITOR_DEF_H_ */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3205540..4465fe8 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -13,6 +13,7 @@ typedef struct AioContext AioContext;
 struct Monitor;
 typedef struct Monitor Monitor;
 typedef struct MigrationParams MigrationParams;
+typedef struct MonitorDef MonitorDef;
 
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..07ad3ee 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -156,6 +156,7 @@ struct kvm_run;
  * @gdb_num_g_regs: Number of registers in GDB 'g' packets.
  * @next_cpu: Next CPU sharing TB cache.
  * @kvm_fd: vCPU file descriptor for KVM.
+ * @monitor_defs array of register definitions for monitor functions
  *
  * State of one CPU core or thread.
  */
@@ -198,6 +199,8 @@ struct CPUState {
 struct KVMState *kvm_state;
 struct kvm_run *kvm_run;
 
+const MonitorDef *monitor_defs;
+
 /* TODO Move common fields from CPUArchState here. */
 int cpu_index; /* used by alpha TCG */
 uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/monitor.c b/monitor.c
index 74f3f1b..ce2c2af 100644
--- a/monitor.c
+++ b/monitor.c
@@ -37,6 +37,7 @@
 #include ui/qemu-spice.h
 #include sysemu/sysemu.h
 #include monitor/monitor.h
+#include monitor/monitor_def.h
 #include monitor/readline.h
 #include ui/console.h
 #include sysemu/blockdev.h
@@ -1096,7 +1097,7 @@ int monitor_set_cpu(int cpu_index)
 return 0;
 }
 
-static CPUArchState *mon_get_cpu(void)
+CPUArchState *mon_get_cpu(void)
 {
 if (!cur_mon-mon_cpu) {
 monitor_set_cpu(0);
@@ -2969,392 +2970,6 @@ static const mon_cmd_t qmp_cmds[] = {
 static const char *pch;
 static sigjmp_buf expr_env;
 
-#define MD_TLONG 0
-#define MD_I32   1
-
-typedef struct MonitorDef {
-const char *name;
-int offset;
-target_long (*get_value)(const struct MonitorDef *md, int val);
-int type;
-} MonitorDef;
-
-#if defined(TARGET_I386)
-static target_long monitor_get_pc (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-eip + env-segs[R_CS].base;
-}
-#endif
-
-#if defined(TARGET_PPC)
-static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-unsigned int u;
-int i;
-
-u = 0;
-for (i = 0; i  8; i++)
-u |= env-crf[i]  (32 - (4 * i));
-
-return u;
-}
-
-static target_long monitor_get_msr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-msr;
-}
-
-static target_long monitor_get_xer (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-xer;
-}
-
-static target_long monitor_get_decr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_decr(env);
-}
-
-static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_tbu(env);
-}
-
-static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_tbl(env);
-}
-#endif
-
-#if defined(TARGET_SPARC)
-#ifndef TARGET_SPARC64
-static target_long monitor_get_psr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env

Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-01 Thread Fabien Chouteau
On 10/01/2013 03:08 AM, Peter Maydell wrote:
 On 1 October 2013 00:57, Fabien Chouteau chout...@adacore.com wrote:
 
 +#define MD_I64 0
 +#define MD_I32 1
 
 -#define MD_TLONG 0
 -#define MD_I32   1
 
 -{ eax, offsetof(CPUX86State, regs[0]) },
 -{ ecx, offsetof(CPUX86State, regs[1]) },
 
 +{ eax, offsetof(CPUX86State, regs[0]) },
 +{ ecx, offsetof(CPUX86State, regs[1]) },
 
 I like this generally, but this detail is wrong. These changes
 mean that these registers (and many others) are now described
 as being int64_t wide rather than target_long wide, so you'll
 find that on 32 bit x86 they will read/write incorrectly.
 This is why I suggested that you need to have target-i386/monitor.c
 do an
 #if TARGET_LONG_BITS == 32
 #define MD_TLONG MD_I32
 #else
 #define MD_TLONG MD_I64
 #endif
 
 and then specifically mark these fields as MD_TLONG.

This seems complicated. Is there a way to use target_long in monitor.h?

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-01 Thread Fabien Chouteau
On 10/01/2013 04:00 AM, Richard Henderson wrote:
 On 09/30/2013 08:57 AM, Fabien Chouteau wrote:
 +extern const MonitorDef arch_monitor_defs[];

 This is supplied by target-foo/monitor.c, right?
 Why in the world is it declared in generic code?


Yes, why?

 Especially if it's only ever accessed via the
 cpu-monitor_defs member?


To begin with, I though I'd put in in each target-*/cpu.c, then having
it at only one place seemed more clean. I'm open to any suggestion.

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-10-01 Thread Fabien Chouteau
On 10/01/2013 05:21 PM, Peter Maydell wrote:
 The declaration should go in cpu-qom.h (again, compare
 the gdb stuff), and it should be called arm_monitor_defs,
 ppc_monitor_defs etc, not arch_monitor_defs. (This
 avoids issues if we ever manage to compile more than one
 target CPU into a single qemu binary, and again, it follows
 existing conventions).
 

monitor.h drag a lot of thing...

In file included from 
/home/chouteau/src/qemu-main/include/monitor/monitor.h:7:0,
 from /home/chouteau/src/qemu-main/target-i386/cpu-qom.h:26,
 from /home/chouteau/src/qemu-main/target-i386/cpu.h:917,
 from /home/chouteau/src/qemu-main/include/qemu-common.h:116,
 from /home/chouteau/src/qemu-main/exec.c:27:
/home/chouteau/src/qemu-main/include/block/block.h:187:59: error: unknown type 
name ‘QEMUIOVector’


Maybe MonitorDef can be in a specific file include/monitor/monitordef.h?
included by monitor.c target-*/monitor.c and target-*/cpu.c

-- 
Fabien Chouteau



[Qemu-devel] [PATCH 4/4] Add ARM registers definitions for Monitor commands

2013-09-30 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-arm/Makefile.objs |2 +-
 target-arm/cpu.c |5 +
 target-arm/monitor.c |   40 
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 target-arm/monitor.c

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 6453f5c..6c3ec32 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -1,5 +1,5 @@
 obj-y += arm-semi.o
-obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_SOFTMMU) += machine.o monitor.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += translate.o op_helper.o helper.o cpu.o
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d40f2a7..78f7dae 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -26,6 +26,7 @@
 #include hw/arm/arm.h
 #include sysemu/sysemu.h
 #include sysemu/kvm.h
+#include monitor/monitor.h
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -217,6 +218,10 @@ static void arm_cpu_initfn(Object *obj)
ARRAY_SIZE(cpu-gt_timer_outputs));
 #endif
 
+#if !defined(CONFIG_USER_ONLY)
+cs-monitor_defs = arch_monitor_defs;
+#endif
+
 if (tcg_enabled()  !inited) {
 inited = true;
 arm_translate_init();
diff --git a/target-arm/monitor.c b/target-arm/monitor.c
new file mode 100644
index 000..7321248
--- /dev/null
+++ b/target-arm/monitor.c
@@ -0,0 +1,40 @@
+/*
+ * ARM MonitorDef
+ *
+ * Copyright (c) 2013 AdaCore
+ *
+ * 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/.
+ */
+
+#include monitor/monitor.h
+
+const MonitorDef arch_monitor_defs[] = {
+{ r0, offsetof(CPUARMState, regs[0])  },
+{ r1, offsetof(CPUARMState, regs[1])  },
+{ r2, offsetof(CPUARMState, regs[2])  },
+{ r3, offsetof(CPUARMState, regs[3])  },
+{ r4, offsetof(CPUARMState, regs[4])  },
+{ r5, offsetof(CPUARMState, regs[5])  },
+{ r6, offsetof(CPUARMState, regs[6])  },
+{ r7, offsetof(CPUARMState, regs[7])  },
+{ r8, offsetof(CPUARMState, regs[8])  },
+{ r9, offsetof(CPUARMState, regs[9])  },
+{ r10,offsetof(CPUARMState, regs[10]) },
+{ r11,offsetof(CPUARMState, regs[11]) },
+{ r12,offsetof(CPUARMState, regs[12]) },
+{ r13|sp, offsetof(CPUARMState, regs[13]) },
+{ r14|lr, offsetof(CPUARMState, regs[14]) },
+{ r15|pc, offsetof(CPUARMState, regs[15]) },
+{ NULL },
+};
-- 
1.7.9.5




[Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array

2013-09-30 Thread Fabien Chouteau
Everything has been moved to cpu specific directories (SPARC, PPC,
i386).

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 include/monitor/monitor.h   |   16 ++
 include/qemu/typedefs.h |1 +
 include/qom/cpu.h   |3 +
 monitor.c   |  399 +--
 target-i386/Makefile.objs   |2 +-
 target-i386/cpu.c   |6 +-
 target-i386/monitor.c   |   63 +++
 target-ppc/Makefile.objs|2 +-
 target-ppc/monitor.c|  234 +
 target-ppc/translate_init.c |5 +
 target-sparc/Makefile.objs  |2 +-
 target-sparc/cpu.c  |5 +
 target-sparc/monitor.c  |  138 +++
 13 files changed, 482 insertions(+), 394 deletions(-)
 create mode 100644 target-i386/monitor.c
 create mode 100644 target-ppc/monitor.c
 create mode 100644 target-sparc/monitor.c

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..208ed76 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -7,6 +7,22 @@
 #include block/block.h
 #include monitor/readline.h
 
+#ifdef NEED_CPU_H
+CPUArchState *mon_get_cpu(void);
+#endif
+
+#define MD_I64 0
+#define MD_I32 1
+
+typedef struct MonitorDef {
+const char *name;
+int offset;
+uint64_t (*get_value)(const struct MonitorDef *md, int val);
+int type;
+} MonitorDef;
+
+extern const MonitorDef arch_monitor_defs[];
+
 extern Monitor *cur_mon;
 extern Monitor *default_mon;
 
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3205540..4465fe8 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -13,6 +13,7 @@ typedef struct AioContext AioContext;
 struct Monitor;
 typedef struct Monitor Monitor;
 typedef struct MigrationParams MigrationParams;
+typedef struct MonitorDef MonitorDef;
 
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..07ad3ee 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -156,6 +156,7 @@ struct kvm_run;
  * @gdb_num_g_regs: Number of registers in GDB 'g' packets.
  * @next_cpu: Next CPU sharing TB cache.
  * @kvm_fd: vCPU file descriptor for KVM.
+ * @monitor_defs array of register definitions for monitor functions
  *
  * State of one CPU core or thread.
  */
@@ -198,6 +199,8 @@ struct CPUState {
 struct KVMState *kvm_state;
 struct kvm_run *kvm_run;
 
+const MonitorDef *monitor_defs;
+
 /* TODO Move common fields from CPUArchState here. */
 int cpu_index; /* used by alpha TCG */
 uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/monitor.c b/monitor.c
index 74f3f1b..059fc72 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1096,7 +1096,7 @@ int monitor_set_cpu(int cpu_index)
 return 0;
 }
 
-static CPUArchState *mon_get_cpu(void)
+CPUArchState *mon_get_cpu(void)
 {
 if (!cur_mon-mon_cpu) {
 monitor_set_cpu(0);
@@ -2969,392 +2969,6 @@ static const mon_cmd_t qmp_cmds[] = {
 static const char *pch;
 static sigjmp_buf expr_env;
 
-#define MD_TLONG 0
-#define MD_I32   1
-
-typedef struct MonitorDef {
-const char *name;
-int offset;
-target_long (*get_value)(const struct MonitorDef *md, int val);
-int type;
-} MonitorDef;
-
-#if defined(TARGET_I386)
-static target_long monitor_get_pc (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-eip + env-segs[R_CS].base;
-}
-#endif
-
-#if defined(TARGET_PPC)
-static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-unsigned int u;
-int i;
-
-u = 0;
-for (i = 0; i  8; i++)
-u |= env-crf[i]  (32 - (4 * i));
-
-return u;
-}
-
-static target_long monitor_get_msr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-msr;
-}
-
-static target_long monitor_get_xer (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-xer;
-}
-
-static target_long monitor_get_decr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_decr(env);
-}
-
-static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_tbu(env);
-}
-
-static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return cpu_ppc_load_tbl(env);
-}
-#endif
-
-#if defined(TARGET_SPARC)
-#ifndef TARGET_SPARC64
-static target_long monitor_get_psr (const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-
-return cpu_get_psr(env);
-}
-#endif
-
-static target_long monitor_get_reg(const struct MonitorDef *md, int val)
-{
-CPUArchState *env = mon_get_cpu();
-return env-regwptr[val];
-}
-#endif
-
-static const MonitorDef monitor_defs[] = {
-#ifdef TARGET_I386

[Qemu-devel] [PATCH 2/4] Improve Monitor disas with symbol lookup

2013-09-30 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 disas.c |   10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index 32407de..c83bf5b 100644
--- a/disas.c
+++ b/disas.c
@@ -507,7 +507,15 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 #endif
 
 for (i = 0; i  nb_insn; i++) {
-monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
+const char *sym = lookup_symbol(pc);
+
+monitor_printf(mon, 0x TARGET_FMT_lx, pc);
+if (sym[0] != '\0') {
+monitor_printf(mon,  %s:  , sym);
+} else {
+monitor_printf(mon, :  );
+}
+
 count = print_insn(pc, s.info);
 monitor_printf(mon, \n);
 if (count  0) {
-- 
1.7.9.5




[Qemu-devel] [PATCH 0/4] Few minor improvements of monitor disas command (v3)

2013-09-30 Thread Fabien Chouteau
I finaly managed to move the target depend code in 'target-*/'.

Fabien Chouteau (4):
  Fix coding style
  Improve Monitor disas with symbol lookup
  Refactoring MonitorDef array
  Add ARM registers definitions for Monitor commands

 disas.c |   19 ++-
 include/monitor/monitor.h   |   16 ++
 include/qemu/typedefs.h |1 +
 include/qom/cpu.h   |3 +
 monitor.c   |  399 +--
 target-arm/Makefile.objs|2 +-
 target-arm/cpu.c|5 +
 target-arm/monitor.c|   40 +
 target-i386/Makefile.objs   |2 +-
 target-i386/cpu.c   |6 +-
 target-i386/monitor.c   |   63 +++
 target-ppc/Makefile.objs|2 +-
 target-ppc/monitor.c|  234 +
 target-ppc/translate_init.c |5 +
 target-sparc/Makefile.objs  |2 +-
 target-sparc/cpu.c  |5 +
 target-sparc/monitor.c  |  138 +++
 17 files changed, 542 insertions(+), 400 deletions(-)
 create mode 100644 target-arm/monitor.c
 create mode 100644 target-i386/monitor.c
 create mode 100644 target-ppc/monitor.c
 create mode 100644 target-sparc/monitor.c

-- 
1.7.9.5




[Qemu-devel] [PATCH 1/4] Fix coding style

2013-09-30 Thread Fabien Chouteau

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 disas.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/disas.c b/disas.c
index 0203ef2..32407de 100644
--- a/disas.c
+++ b/disas.c
@@ -506,12 +506,13 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 return;
 #endif
 
-for(i = 0; i  nb_insn; i++) {
-   monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
+for (i = 0; i  nb_insn; i++) {
+monitor_printf(mon, 0x TARGET_FMT_lx :  , pc);
 count = print_insn(pc, s.info);
-   monitor_printf(mon, \n);
-   if (count  0)
-   break;
+monitor_printf(mon, \n);
+if (count  0) {
+break;
+}
 pc += count;
 }
 }
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH 3/3] Add ARM registers definitions in Monitor commands

2013-09-26 Thread Fabien Chouteau
On 09/26/2013 02:05 AM, Peter Maydell wrote:
 On 26 September 2013 01:29, Fabien Chouteau chout...@adacore.com wrote:
 On 09/25/2013 05:51 PM, Peter Maydell wrote:
 On 26 September 2013 00:38, Fabien Chouteau chout...@adacore.com wrote:
 It doesn't matter very much, but monitor.h seems the obvious
 place. You probably don't want qom/cpu.h to have to drag in
 monitor.h so a 'struct MonitorDef;' forward declaration in cpu.h
 will let you avoid that (we do that already for a few other structs).

 I think that's what I did. I think the problem was to include
 'monitor.h' in 'target-*/cpu.c'.
 
 Why doesn't that work?
 

The problem is use of 'target_long' in 'monitor.h'.


-- 
Fabien Chouteau



  1   2   3   4   5   >