Re: [Qemu-devel] [V6, 2/7] nios2: Add architecture emulation support

[Marek Vasut]

On 11/13/2016 05:25 PM, Guenter Roeck wrote:
> On 11/13/2016 04:43 AM, Marek Vasut wrote:
>> On 11/13/2016 01:01 PM, Marek Vasut wrote:
>>> On 11/13/2016 12:25 AM, Guenter Roeck wrote:
>>>> Hi Marek,
>>>
>>> Hi!
>>>
>>>> On 11/12/2016 01:50 PM, Marek Vasut wrote:
>>>>> On 11/07/2016 08:54 PM, Guenter Roeck wrote:
>>>>>> Hi Marek,
>>>>>>
>>>>>> On 11/07/2016 10:14 AM, Marek Vasut wrote:
>>>>>>> On 11/07/2016 04:58 AM, Guenter Roeck wrote:
>>>>>>>> On Tue, Oct 25, 2016 at 09:57:43PM +0200, Marek Vasut wrote:
>>>>>>>>> From: Chris Wulff <address@hidden>
>>>>>>>>>
>>>>>>>>> Add support for emulating Altera NiosII R1 architecture into qemu.
>>>>>>>>> This patch is based on previous work by Chris Wulff from 2012 and
>>>>>>>>> updated to latest mainline QEMU.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Marek Vasut <address@hidden>
>>>>>>>>> Cc: Chris Wulff <address@hidden>
>>>>>>>>> Cc: Jeff Da Silva <address@hidden>
>>>>>>>>> Cc: Ley Foon Tan <address@hidden>
>>>>>>>>> Cc: Sandra Loosemore <address@hidden>
>>>>>>>>> Cc: Yves Vandervennet <address@hidden>
>>>>>>>>> ---
>>>>>>>>> V3: Thorough cleanup, deal with the review comments all over the
>>>>>>>>> place
>>>>>>>>> V4: - Use extract32()
>>>>>>>>>     - Fix gen_goto_tb() , suppress tcg_gen_goto_tb()
>>>>>>>>>     - Clean up gen_check_supervisor() helper
>>>>>>>>>     - Use TCGMemOp type for flags
>>>>>>>>>     - Drop jump labels from wrctl/rdctl
>>>>>>>>>     - More TCG cleanup
>>>>>>>>> V5: - Simplify load/store handling
>>>>>>>>>     - Handle loads into R_ZERO from protected page, add comment
>>>>>>>>> V6: - Fix division opcode handling
>>>>>>>>>     - Add missing disas handling
>>>>>>>>>     - V5 review comments cleanup
>>>>>>>>> ---
>>>>>>>> [ ... ]
>>>>>>>>
>>>>>>>>> diff --git a/target-nios2/cpu.h b/target-nios2/cpu.h
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..17c9a0f
>>>>>>>> [ ... ]
>>>>>>>>
>>>>>>>>> +static inline void cpu_get_tb_cpu_state(CPUNios2State *env,
>>>>>>>>> target_ulong *pc,
>>>>>>>>> +                                        target_ulong *cs_base,
>>>>>>>>> uint32_t *flags)
>>>>>>>>> +{
>>>>>>>>> +    *pc = env->regs[R_PC];
>>>>>>>>> +    *cs_base = 0;
>>>>>>>>> +    *flags = (env->regs[CR_STATUS] & (CR_STATUS_EH |
>>>>>>>>> CR_STATUS_U));
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +#endif /* CPU_NIOS2_H */
>>>>>>>>> +
>>>>>>>>
>>>>>>>> The empty line at the end results in a whitespace message from git.
>>>>>>>
>>>>>>> Dropped, thanks. Is there anything else or is this patchset
>>>>>>> starting to
>>>>>>> become acceptable ?
>>>>>>>
>>>>>>
>>>>>> Hard for me to say. I tried to build and run the series with the
>>>>>> latest
>>>>>> linux
>>>>>> kernel (v4.9-rc4), but it is stuck in early boot. I tried with
>>>>>> 10m50_defconfig
>>>>>> and 10m50_devboard.dtb. gcc is 6.1.0 built with buildroot, though
>>>>>> I also
>>>>>> tried
>>>>>> with toolchains from CodeSourcery. Obviously I have no idea if there
>>>>>> is a
>>>>>> kernel bug or a qemu bug or a problem with the command line I used.
>>>>>>
>>>>>> Here is my command line:
>>>>>>
>>>>>> qemu-system-nios2 -M 10m50-ghrd -kernel vmlinux -dtb
>>>>>> 10m50_devboard.dtb \
>>>>>>     -append "earlycon=uart8250,mmio32,0x18001600,115200n8
>>>>>> console=ttyS0"
>>>>>>
>>>>>> This may be wrong, but the boot is stuck in an endless loop in
>>>>>> mark_bootmem(),
>>>>>> which seems early and odd. I tried with both vmlinux and
>>>>>> arch/nios2/boot/vmImage,
>>>>>> with the same results.
>>>>>>
>>>>>> Can you provide a working command line and kernel version, and/or
>>>>>> directions how
>>>>>> to create a working image if I need to run the image, for example,
>>>>>> from
>>>>>> u-boot ?
>>>>>> Sorry if that is posted somewhere and I missed it.
>>>>>
>>>>> I guess Romain gave you something since I see you made some progress.
>>>>> I'll have to look into that fdt loader issue.
>>>>>
>>>>
>>>> Yes, WFM after
>>>>
>>>> diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
>>>> index 564dbae..e0a9aff 100644
>>>> --- a/hw/nios2/boot.c
>>>> +++ b/hw/nios2/boot.c
>>>> @@ -73,6 +73,11 @@ static void main_cpu_reset(void *opaque)
>>>>      }
>>>>  }
>>>>
>>>> +static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>>>> +{
>>>> +    return addr - 0xc0000000LL;
>>>> +}
>>>> +
>>>>  static int nios2_load_dtb(struct nios2_boot_info bi, const uint32_t
>>>> ramsize,
>>>>                            const char *kernel_cmdline, const char
>>>> *dtb_filename)
>>>>  {
>>>> @@ -97,21 +102,16 @@ static int nios2_load_dtb(struct nios2_boot_info
>>>> bi, const uint32_t ramsize,
>>>>
>>>>      if (bi.initrd_start) {
>>>>          qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
>>>> -                              bi.initrd_start);
>>>> +                              translate_kernel_address(NULL,
>>>> bi.initrd_start));
>>>>
>>>>          qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
>>>> -                              bi.initrd_end);
>>>> +                              translate_kernel_address(NULL,
>>>> bi.initrd_end));
>>>>      }
>>>>
>>>>      cpu_physical_memory_write(bi.fdt, fdt, fdt_size);
>>>>      return fdt_size;
>>>>  }
>>>>
>>>> -static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>>>> -{
>>>> -    return addr - 0xc0000000LL;
>>>> -}
>>>> -
>>>>  void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
>>>>
>>>>
>>>> This is my command line, running your patch series plus the above fixup
>>>> on top of
>>>> the qemu master branch:
>>>
>>> Uhm, how could that patch fix the DT passing for you ? I am still
>>> looking into the DT passing, that's quite odd.
>>>
>> This patch makes the DT passing work for me, but I cannot put my finger
>> on why. It moves the DT a bit further in memory, that's all.
>>
>> diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
>> index 564dbae..c9e3e69 100644
>> --- a/hw/nios2/boot.c
>> +++ b/hw/nios2/boot.c
>> @@ -178,7 +183,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr
>> ddr_base,
>>              high = ddr_base + kernel_size;
>>          }
>>
>> -        high = ROUND_UP(high, 1024 * 1024);
>> +        high = ROUND_UP(high, 1024 * 1024) + 0x10000;
>>
>>          /* If initrd is available, it goes after the kernel, aligned to
>> 1M. */
>>          if (initrd_filename) {
>>
>>
> 
> That doesn't work for me, and I would be quite surprised if it would.
> The initrd
> addresses are still wrong, still causing the kernel to hang.

I'm not using initrd , so your change doesn't impact me. But that's a
good point, I'll wrap it in if you don't mind.

> It does appear to work if I don't pass the -initrd argument, but that
> misses
> the point. I just checked, it is the -initrd argument that makes it hang
> in the first place, not the -dtb argument. Sorry if that created confusion
> earlier.

I think we have two problems here :-( The DTB problem seems to happen
even on real hardware though, so ... damn ... there's something to be
fixed in the kernel for sure. And for whatever reason, loading the DTB
to offset +0x10000 (or further) makes things work, always, which might
be a hint for where to look for this issue in the kernel.

-- 
Best regards,
Marek Vasut