diff mbox series

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

Message ID 20201221114314.25588-3-sughosh.ganu@linaro.org
State New
Headers show
Series qemu: arm64: Add support for uefi capsule update on qemu arm platform | expand

Commit Message

Sughosh Ganu Dec. 21, 2020, 11:43 a.m. UTC 
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@linaro.org>

---

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(+)

-- 
2.17.1

Comments

Heinrich Schuchardt Dec. 21, 2020, 12:19 p.m. UTC | #1 
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@linaro.org>


With which commands can I see the difference before and after the patch?

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)

>   {

>   	/*

>   	 * 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

>   CONFIG_PCI_INIT_R=y

>   CONFIG_CMD_BOOTEFI_SELFTEST=y

>   CONFIG_CMD_NVEDIT_EFI=y

>
Heinrich Schuchardt Dec. 21, 2020, 12:51 p.m. UTC | #2 
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@linaro.org>

>

> With which commands can I see the difference before and after the patch?


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.

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

>>   {

>>       /*

>>        * 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!

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

Best regards

Heinrich


>>   CONFIG_PCI_INIT_R=y

>>   CONFIG_CMD_BOOTEFI_SELFTEST=y

>>   CONFIG_CMD_NVEDIT_EFI=y

>>

>
Sughosh Ganu Dec. 21, 2020, 5:18 p.m. UTC | #3 
On Mon, 21 Dec 2020 at 18:21, Heinrich Schuchardt <xypron.glpk@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@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
diff mbox series

Patch

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)
 {
 	/*
 	 * 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
 CONFIG_PCI_INIT_R=y
 CONFIG_CMD_BOOTEFI_SELFTEST=y
 CONFIG_CMD_NVEDIT_EFI=y