[Qemu-devel] [PATCH 0/5] hw/arm/boot: Support DTB autoload for firmware-only boots

Peter Maydell posted 5 patches 5 years, 1 month ago
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190131112240.8395-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/arm/boot.c | 166 +++++++++++++++++++++++++++++---------------------
1 file changed, 96 insertions(+), 70 deletions(-)
[Qemu-devel] [PATCH 0/5] hw/arm/boot: Support DTB autoload for firmware-only boots
Posted by Peter Maydell 5 years, 1 month ago
The arm_boot_info struct has a skip_dtb_autoload flag: if this is
set to true by the board code then arm_load_kernel() will not
load the DTB itself, but will leave this for the board code to
do itself later. However, the check for this is done in a
code path which is only executed for the case where we load
a kernel image file. If we're taking the "boot via firmware"
code path then the flag isn't honoured and the DTB is never
loaded.
    
We didn't notice this because the only real user of "boot
via firmware" that cares about the DTB is the virt board
(for UEFI boot), and that always wants skip_dtb_autoload
anyway. But the SBSA reference board model we're planning to
add will want the flag to behave correctly.

The first four patches in this set are just refactoring,
splitting the code in arm_load_kernel() out into a
couple of sub-functions for "direct kernel boot" and
"firmware boot". The last patch that fixes the issue is
then just a one-liner. (Without the refactoring we'd
need to use a goto, which is what I wanted to avoid.)

thanks
-- PMM

Peter Maydell (5):
  hw/arm/boot: Fix block comment style in arm_load_kernel()
  hw/arm/boot: Factor out "direct kernel boot" code into its own
    function
  hw/arm/boot: Factor out "set up firmware boot" code
  hw/arm/boot: Clarify why arm_setup_firmware_boot() doesn't set
    env->boot_info
  hw/arm/boot: Support DTB autoload for firmware-only boots

 hw/arm/boot.c | 166 +++++++++++++++++++++++++++++---------------------
 1 file changed, 96 insertions(+), 70 deletions(-)

-- 
2.20.1


Re: [Qemu-devel] [PATCH 0/5] hw/arm/boot: Support DTB autoload for firmware-only boots
Posted by Richard Henderson 5 years, 1 month ago
On 1/31/19 3:22 AM, Peter Maydell wrote:
> Peter Maydell (5):
>   hw/arm/boot: Fix block comment style in arm_load_kernel()
>   hw/arm/boot: Factor out "direct kernel boot" code into its own
>     function
>   hw/arm/boot: Factor out "set up firmware boot" code
>   hw/arm/boot: Clarify why arm_setup_firmware_boot() doesn't set
>     env->boot_info
>   hw/arm/boot: Support DTB autoload for firmware-only boots
> 
>  hw/arm/boot.c | 166 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 96 insertions(+), 70 deletions(-)

Nice cleanup.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 0/5] hw/arm/boot: Support DTB autoload for firmware-only boots
Posted by Igor Mammedov 5 years, 1 month ago
On Thu, 31 Jan 2019 11:22:35 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> The arm_boot_info struct has a skip_dtb_autoload flag: if this is
> set to true by the board code then arm_load_kernel() will not
> load the DTB itself, but will leave this for the board code to
> do itself later. However, the check for this is done in a
> code path which is only executed for the case where we load
> a kernel image file. If we're taking the "boot via firmware"
> code path then the flag isn't honoured and the DTB is never
> loaded.
>     
> We didn't notice this because the only real user of "boot
> via firmware" that cares about the DTB is the virt board
> (for UEFI boot), and that always wants skip_dtb_autoload
> anyway. But the SBSA reference board model we're planning to
> add will want the flag to behave correctly.
> 
> The first four patches in this set are just refactoring,
> splitting the code in arm_load_kernel() out into a
> couple of sub-functions for "direct kernel boot" and
> "firmware boot". The last patch that fixes the issue is
> then just a one-liner. (Without the refactoring we'd
> need to use a goto, which is what I wanted to avoid.)
> 
> thanks
> -- PMM
> 
> Peter Maydell (5):
>   hw/arm/boot: Fix block comment style in arm_load_kernel()
>   hw/arm/boot: Factor out "direct kernel boot" code into its own
>     function
>   hw/arm/boot: Factor out "set up firmware boot" code
>   hw/arm/boot: Clarify why arm_setup_firmware_boot() doesn't set
>     env->boot_info
>   hw/arm/boot: Support DTB autoload for firmware-only boots
> 
>  hw/arm/boot.c | 166 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 96 insertions(+), 70 deletions(-)
> 

Reviewed-by: Igor Mammedov <imammedo@redhat.com>