Re: [Qemu-arm] [PATCH v3 0/5] arm: isolate and clean up dtb generation

[Igor Mammedov]

On Fri, 4 May 2018 17:28:55 +0100
Peter Maydell <address@hidden> wrote:

> On 3 May 2018 at 16:03, Peter Maydell <address@hidden> wrote:
> > On 1 May 2018 at 13:08, Igor Mammedov <address@hidden> wrote:  
> >>
> >> Changelog v2->v3:
> >>    * drop already merged patches
> >>        'arm: always start from  first_cpu when registering loader cpu 
> >> reset callback'
> >>        'ppc: e500: switch E500 based  machines to full machine definition'
> >>    * add small not related cleanup
> >>        'arm: boot: set boot_info starting from first_cpu'
> >>    * add extra suggested patch
> >>        'make sure that we aren't overwriting mc->get_hotplug_handler by 
> >> accident'
> >>    * make sure that dtb_limit initialized to 0
> >>    * drop stale comment
> >>    * drop not needed line movement
> >>    * drop not needed extra new line
> >> Changelog v1->v2:
> >>    * drop "arm: reuse  arm_boot_address_space() in armv7m_load_kernel()"
> >>    * move "arm: always start from first_cpu  when registering loader cpu 
> >> reset callback"
> >>      at the begigning of series and rebase
> >>    * add "ppc: e500: switch E500 based machines to full  machine 
> >> definition"
> >>      and rebase 4/5 on top of it
> >>    * fixup typo in virt_machine_get_hotpug_handler() name
> >>    * add doc comment to skip_dtb_autoload field
> >>    * 1-2/5 are queued in respective arm/ppc trees and are included for 
> >> series
> >>      completness so it would be easier to test, I expect series to go 
> >> through
> >>      arm tree
> >>  
> >
> > Applied to target-arm.next, thanks.  
> 
> Doing further testing within target-arm.next shows that this series (and
> specifically patch 3/5) causes segfaults for the "no DTB provided" case.
> 
> $ gdb --args ./build/x86/arm-softmmu/qemu-system-arm -M vexpress-a15
[...]

> arm_load_dtb() is trying to call the binfo->get_dtb() hook and is not
> handling the case where it is NULL (which it is on pretty much every
> board except "virt").
> 
> (You get a segfault with an actual guest kernel as well; I just used
> /dev/null here as an easy no-dependencies-required repro case.)
> 
> I've left patch 4 in target-arm.next as that was a standalone bugfix,
> but have dropped the rest of the series for now.

Fix is really trivial, I've lost have_dtb() guard, while consolidating
multiple load_dtb() calls into one.
I'll respin v4 shortly with fixed 3/5.


> thanks
> -- PMM