[PATCH v2 02/14] qemu: arm: Initialise virtio in board_late_init

[Sughosh Ganu]

On Mon, 21 Dec 2020 at 18:21, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> On 12/21/20 1:19 PM, Heinrich Schuchardt wrote:
> > On 12/21/20 12:43 PM, Sughosh Ganu wrote:
> >> On the qemu arm platform, the virtio devices are initialised in
> >> board_init, which gets called before the initr_pci. With this, the
> >> virtio block devices on the pci bus are not initialised. Move the
> >> initialisation of virtio devices to board_late_init which gets called
> >> after the call to initr_pci.
> >>
> >> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> >
> > With which commands can I see the difference before and after the patch?
>

Without this patch, i am required to run a 'virtio scan' on the command
line to access my efi system partition which is a block device on the pci
bus. Do you see any issue with moving the virtio_init in board_late_init?


>
> The 'rng' command now works without calling 'virtio scan' beforehand.


> >
> > Best regards
> >
> > Heinrich
> >
> >> ---
> >>
> >> Changes since V1:
> >> * The earlier patch was adding a call to pci_init in board_init. Moved
> >>    the virtio_init call to board_late_init
> >>
> >>   board/emulation/qemu-arm/qemu-arm.c | 5 +++++
> >>   configs/qemu_arm64_defconfig        | 1 +
> >>   2 files changed, 6 insertions(+)
> >>
> >> diff --git a/board/emulation/qemu-arm/qemu-arm.c
> >> b/board/emulation/qemu-arm/qemu-arm.c
> >> index f18f2ed7da..aa68bef469 100644
> >> --- a/board/emulation/qemu-arm/qemu-arm.c
> >> +++ b/board/emulation/qemu-arm/qemu-arm.c
> >> @@ -64,6 +64,11 @@ struct mm_region *mem_map = qemu_arm64_mem_map;
> >>   #endif
> >>
> >>   int board_init(void)
> >> +{
> >> +    return 0;
> >> +}
> >> +
> >> +int board_late_init(void)
>
> Why don't you change board/emulation/qemu-riscv/qemu-riscv.c too? I can
> see no reason why you want to treat RISC-V differently.
>

Like I mentioned in my other mail, I am testing the capsule update feature
only on the qemu arm64 platform. I can make this change, but I think this
should be done by someone who is actually a user of the qemu risc-v
platform.


> What about x86 and MIPS? Why is virtio_init() not called on those
> architectures?
>

I have no idea about this :-)


> >>   {
> >>       /*
> >>        * Make sure virtio bus is enumerated so that peripherals
> >> diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
> >> index f6e586627a..5c855fa08c 100644
> >> --- a/configs/qemu_arm64_defconfig
> >> +++ b/configs/qemu_arm64_defconfig
> >> @@ -14,6 +14,7 @@ CONFIG_LEGACY_IMAGE_FORMAT=y
> >>   CONFIG_USE_PREBOOT=y
> >>   # CONFIG_DISPLAY_CPUINFO is not set
> >>   # CONFIG_DISPLAY_BOARDINFO is not set
> >> +CONFIG_BOARD_LATE_INIT=y
>
>
> The C code change concerns both arm64 and arm. So on all ARM QEMU boards
> except qemu_arm64_defconfig you don't call virtio_init() at all once
> this patch is applied. This cannot be correct!
>

You are right. I should be adding this in the qemu arm platform flavour as
well along with the arm64 variant. Will fix.


>
> Probably you want to change arch/Kconfig instead:
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e8f9a9e1b7..1c66743ab6 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -24,6 +24,7 @@ config ARM
>          bool "ARM architecture"
>          select CREATE_ARCH_SYMLINK
>          select HAVE_PRIVATE_LIBGCC if !ARM64
> +       select BOARD_LATE_INIT
>          select SUPPORT_OF_CONTROL
>

Will check this out. Thanks.

-sughosh