[Qemu-devel] clk_setup decl missing in virtex_init

[Qemu-devel] clk_setup decl missing in virtex_init

[Michael Eager]

The declaration of clk_setup is missing in
qemu/hw/virtex_ml507.c:

static void virtex_init(ram_addr_t ram_size,
                         const char *boot_device,
                         const char *kernel_filename,
                         const char *kernel_cmdline,
                         const char *initrd_filename, const char *cpu_model)
{
...
<no decl for clk_setup>
...
     memset(clk_setup, 0, sizeof(clk_setup));

This SEGVs because clk_setup is an inline function defined in ppc.h.
(I presume that the linker generates an out-of-line copy.)

It isn't clear what the declaration should be.  In ppc405_uc.c, there
is a decl:
    clk_setup_t clk_setup[PPC405EP_CLK_NB];


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [Qemu-devel] clk_setup decl missing in virtex_init

[Edgar E. Iglesias]

On Mon, Nov 07, 2011 at 02:46:47PM -0800, Michael Eager wrote:
> The declaration of clk_setup is missing in
> qemu/hw/virtex_ml507.c:
> 
> static void virtex_init(ram_addr_t ram_size,
>                         const char *boot_device,
>                         const char *kernel_filename,
>                         const char *kernel_cmdline,
>                         const char *initrd_filename, const char *cpu_model)
> {
> ...
> <no decl for clk_setup>
> ...
>     memset(clk_setup, 0, sizeof(clk_setup));
> 
> This SEGVs because clk_setup is an inline function defined in ppc.h.
> (I presume that the linker generates an out-of-line copy.)
> 
> It isn't clear what the declaration should be.  In ppc405_uc.c, there
> is a decl:
>    clk_setup_t clk_setup[PPC405EP_CLK_NB];


Hi

the following patch seems to work on my side. Fabien, could
you please see if this was your intention? It seems to be
commit ddd1055b07fdfe488a22c2275adaca75f4206d30 that
introduced the segfault.

Cheers

commit 8e95771e4afb6e91c30a53943118afef4631b919
Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Date:   Tue Nov 8 00:00:55 2011 +0100

    virtex: Remove memset of clk_setup
    
    clk_setup is now a function.
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c
index d31a204..5ea0e60 100644
--- a/hw/virtex_ml507.c
+++ b/hw/virtex_ml507.c
@@ -202,7 +202,6 @@ static void virtex_init(ram_addr_t ram_size,
         cpu_model = "440-Xilinx";
     }
 
-    memset(clk_setup, 0, sizeof(clk_setup));
     env = ppc440_init_xilinx(&ram_size, 1, cpu_model, 400000000);
     qemu_register_reset(main_cpu_reset, env);
 

Re: [Qemu-devel] clk_setup decl missing in virtex_init

[Michael Eager]

Works for me.

On 11/07/2011 03:05 PM, Edgar E. Iglesias wrote:
> On Mon, Nov 07, 2011 at 02:46:47PM -0800, Michael Eager wrote:
>> The declaration of clk_setup is missing in
>> qemu/hw/virtex_ml507.c:
>>
>> static void virtex_init(ram_addr_t ram_size,
>>                          const char *boot_device,
>>                          const char *kernel_filename,
>>                          const char *kernel_cmdline,
>>                          const char *initrd_filename, const char *cpu_model)
>> {
>> ...
>> <no decl for clk_setup>
>> ...
>>      memset(clk_setup, 0, sizeof(clk_setup));
>>
>> This SEGVs because clk_setup is an inline function defined in ppc.h.
>> (I presume that the linker generates an out-of-line copy.)
>>
>> It isn't clear what the declaration should be.  In ppc405_uc.c, there
>> is a decl:
>>     clk_setup_t clk_setup[PPC405EP_CLK_NB];
>
>
> Hi
>
> the following patch seems to work on my side. Fabien, could
> you please see if this was your intention? It seems to be
> commit ddd1055b07fdfe488a22c2275adaca75f4206d30 that
> introduced the segfault.
>
> Cheers
>
> commit 8e95771e4afb6e91c30a53943118afef4631b919
> Author: Edgar E. Iglesias<edgar.iglesias@gmail.com>
> Date:   Tue Nov 8 00:00:55 2011 +0100
>
>      virtex: Remove memset of clk_setup
>
>      clk_setup is now a function.
>
>      Signed-off-by: Edgar E. Iglesias<edgar.iglesias@gmail.com>
>
> diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c
> index d31a204..5ea0e60 100644
> --- a/hw/virtex_ml507.c
> +++ b/hw/virtex_ml507.c
> @@ -202,7 +202,6 @@ static void virtex_init(ram_addr_t ram_size,
>           cpu_model = "440-Xilinx";
>       }
>
> -    memset(clk_setup, 0, sizeof(clk_setup));
>       env = ppc440_init_xilinx(&ram_size, 1, cpu_model, 400000000);
>       qemu_register_reset(main_cpu_reset, env);
>
>


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [Qemu-devel] clk_setup decl missing in virtex_init

[Edgar E. Iglesias]

On Mon, Nov 07, 2011 at 03:25:45PM -0800, Michael Eager wrote:
> Works for me.

Thanks, I've applied the fix.

Cheers

Re: [Qemu-devel] clk_setup decl missing in virtex_init

[Fabien Chouteau]

On 08/11/2011 00:05, Edgar E. Iglesias wrote:
> On Mon, Nov 07, 2011 at 02:46:47PM -0800, Michael Eager wrote:
>> The declaration of clk_setup is missing in
>> qemu/hw/virtex_ml507.c:
>>
>> static void virtex_init(ram_addr_t ram_size,
>>                         const char *boot_device,
>>                         const char *kernel_filename,
>>                         const char *kernel_cmdline,
>>                         const char *initrd_filename, const char *cpu_model)
>> {
>> ...
>> <no decl for clk_setup>
>> ...
>>     memset(clk_setup, 0, sizeof(clk_setup));
>>
>> This SEGVs because clk_setup is an inline function defined in ppc.h.
>> (I presume that the linker generates an out-of-line copy.)
>>
>> It isn't clear what the declaration should be.  In ppc405_uc.c, there
>> is a decl:
>>    clk_setup_t clk_setup[PPC405EP_CLK_NB];
> 
> 
> Hi
> 
> the following patch seems to work on my side. Fabien, could
> you please see if this was your intention? It seems to be
> commit ddd1055b07fdfe488a22c2275adaca75f4206d30 that
> introduced the segfault.
> 

That's right the memset should have been removed in my patch.

Thanks,

-- 
Fabien Chouteau