The overhead for the OpenBMC firmware images using the a custom U-Boot
is around 2 seconds, which is fine, but with a U-Boot from mainline,
it takes an extra 50 seconds or so to reach Linux. A quick survey on
the number of reads performed on the flash memory region gives the
following figures :
OpenBMC U-Boot 922478 (~ 3.5 MBytes)
Mainline U-Boot 20569977 (~ 80 MBytes)
QEMU must be trashing the TCG TBs and reloading text very often. Some
addresses are read more than 250.000 times. Until we find a solution
to improve boot time, execution from MMIO is not activated by default.
Setting this option also breaks migration compatibility.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/aspeed.h | 2 ++
hw/arm/aspeed.c | 44 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 4423cd0cda71..18521484b90e 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -19,6 +19,8 @@ typedef struct AspeedBoardState AspeedBoardState;
typedef struct AspeedMachine {
MachineState parent_obj;
+
+ bool mmio_exec;
} AspeedMachine;
#define ASPEED_MACHINE_CLASS(klass) \
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 0a7dfd29868b..bf23579fa53d 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -260,11 +260,18 @@ static void aspeed_machine_init(MachineState *machine)
* SoC and 128MB for the AST2500 SoC, which is twice as big as
* needed by the flash modules of the Aspeed machines.
*/
- memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
- fl->size, &error_abort);
- memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
- boot_rom);
- write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
+ if (ASPEED_MACHINE(machine)->mmio_exec) {
+ memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+ &fl->mmio, 0, fl->size);
+ memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
+ boot_rom);
+ } else {
+ memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+ fl->size, &error_abort);
+ memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
+ boot_rom);
+ write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
+ }
}
aspeed_board_binfo.ram_size = ram_size;
@@ -398,6 +405,30 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
/* Bus 11: TODO ucd90160@64 */
}
+static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
+{
+ return ASPEED_MACHINE(obj)->mmio_exec;
+}
+
+static void aspeed_set_mmio_exec(Object *obj, bool value, Error **errp)
+{
+ ASPEED_MACHINE(obj)->mmio_exec = value;
+}
+
+static void aspeed_machine_instance_init(Object *obj)
+{
+ ASPEED_MACHINE(obj)->mmio_exec = false;
+}
+
+static void aspeed_machine_class_props_init(ObjectClass *oc)
+{
+ object_class_property_add_bool(oc, "execute-in-place",
+ aspeed_get_mmio_exec,
+ aspeed_set_mmio_exec, &error_abort);
+ object_class_property_set_description(oc, "execute-in-place",
+ "boot directly from CE0 flash device", &error_abort);
+}
+
static void aspeed_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -407,6 +438,8 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data)
mc->no_floppy = 1;
mc->no_cdrom = 1;
mc->no_parallel = 1;
+
+ aspeed_machine_class_props_init(oc);
}
static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
@@ -549,6 +582,7 @@ static const TypeInfo aspeed_machine_types[] = {
.name = TYPE_ASPEED_MACHINE,
.parent = TYPE_MACHINE,
.instance_size = sizeof(AspeedMachine),
+ .instance_init = aspeed_machine_instance_init,
.class_size = sizeof(AspeedMachineClass),
.class_init = aspeed_machine_class_init,
.abstract = true,
--
2.21.1
On 1/7/20 8:34 AM, Cédric Le Goater wrote:
> The overhead for the OpenBMC firmware images using the a custom U-Boot
> is around 2 seconds, which is fine, but with a U-Boot from mainline,
> it takes an extra 50 seconds or so to reach Linux. A quick survey on
> the number of reads performed on the flash memory region gives the
> following figures :
>
> OpenBMC U-Boot 922478 (~ 3.5 MBytes)
> Mainline U-Boot 20569977 (~ 80 MBytes)
>
> QEMU must be trashing the TCG TBs and reloading text very often. Some
> addresses are read more than 250.000 times. Until we find a solution
> to improve boot time, execution from MMIO is not activated by default.
>
> Setting this option also breaks migration compatibility.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/arm/aspeed.h | 2 ++
> hw/arm/aspeed.c | 44 ++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 4423cd0cda71..18521484b90e 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -19,6 +19,8 @@ typedef struct AspeedBoardState AspeedBoardState;
>
> typedef struct AspeedMachine {
> MachineState parent_obj;
> +
> + bool mmio_exec;
> } AspeedMachine;
>
> #define ASPEED_MACHINE_CLASS(klass) \
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 0a7dfd29868b..bf23579fa53d 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -260,11 +260,18 @@ static void aspeed_machine_init(MachineState *machine)
> * SoC and 128MB for the AST2500 SoC, which is twice as big as
> * needed by the flash modules of the Aspeed machines.
> */
> - memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
> - fl->size, &error_abort);
> - memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
> - boot_rom);
> - write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
> + if (ASPEED_MACHINE(machine)->mmio_exec) {
> + memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
> + &fl->mmio, 0, fl->size);
> + memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
> + boot_rom);
> + } else {
> + memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
> + fl->size, &error_abort);
> + memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
> + boot_rom);
> + write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
> + }
Nitpick: I'd use rom_add_file_mr() in write_boot_rom(), and keep the
memory_region_add_subregion() call after the if/else statement.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> }
>
> aspeed_board_binfo.ram_size = ram_size;
> @@ -398,6 +405,30 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
> /* Bus 11: TODO ucd90160@64 */
> }
>
> +static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
> +{
> + return ASPEED_MACHINE(obj)->mmio_exec;
> +}
> +
> +static void aspeed_set_mmio_exec(Object *obj, bool value, Error **errp)
> +{
> + ASPEED_MACHINE(obj)->mmio_exec = value;
> +}
> +
> +static void aspeed_machine_instance_init(Object *obj)
> +{
> + ASPEED_MACHINE(obj)->mmio_exec = false;
> +}
> +
> +static void aspeed_machine_class_props_init(ObjectClass *oc)
> +{
> + object_class_property_add_bool(oc, "execute-in-place",
> + aspeed_get_mmio_exec,
> + aspeed_set_mmio_exec, &error_abort);
> + object_class_property_set_description(oc, "execute-in-place",
> + "boot directly from CE0 flash device", &error_abort);
> +}
> +
> static void aspeed_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -407,6 +438,8 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data)
> mc->no_floppy = 1;
> mc->no_cdrom = 1;
> mc->no_parallel = 1;
> +
> + aspeed_machine_class_props_init(oc);
> }
>
> static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
> @@ -549,6 +582,7 @@ static const TypeInfo aspeed_machine_types[] = {
> .name = TYPE_ASPEED_MACHINE,
> .parent = TYPE_MACHINE,
> .instance_size = sizeof(AspeedMachine),
> + .instance_init = aspeed_machine_instance_init,
> .class_size = sizeof(AspeedMachineClass),
> .class_init = aspeed_machine_class_init,
> .abstract = true,
>
On 1/7/20 9:34 AM, Philippe Mathieu-Daudé wrote:
> On 1/7/20 8:34 AM, Cédric Le Goater wrote:
>> The overhead for the OpenBMC firmware images using the a custom U-Boot
>> is around 2 seconds, which is fine, but with a U-Boot from mainline,
>> it takes an extra 50 seconds or so to reach Linux. A quick survey on
>> the number of reads performed on the flash memory region gives the
>> following figures :
>>
>> OpenBMC U-Boot 922478 (~ 3.5 MBytes)
>> Mainline U-Boot 20569977 (~ 80 MBytes)
>>
>> QEMU must be trashing the TCG TBs and reloading text very often. Some
>> addresses are read more than 250.000 times. Until we find a solution
>> to improve boot time, execution from MMIO is not activated by default.
>>
>> Setting this option also breaks migration compatibility.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> include/hw/arm/aspeed.h | 2 ++
>> hw/arm/aspeed.c | 44 ++++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 41 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
>> index 4423cd0cda71..18521484b90e 100644
>> --- a/include/hw/arm/aspeed.h
>> +++ b/include/hw/arm/aspeed.h
>> @@ -19,6 +19,8 @@ typedef struct AspeedBoardState AspeedBoardState;
>> typedef struct AspeedMachine {
>> MachineState parent_obj;
>> +
>> + bool mmio_exec;
>> } AspeedMachine;
>> #define ASPEED_MACHINE_CLASS(klass) \
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 0a7dfd29868b..bf23579fa53d 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -260,11 +260,18 @@ static void aspeed_machine_init(MachineState *machine)
>> * SoC and 128MB for the AST2500 SoC, which is twice as big as
>> * needed by the flash modules of the Aspeed machines.
>> */
>> - memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
>> - fl->size, &error_abort);
>> - memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>> - boot_rom);
>> - write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
>> + if (ASPEED_MACHINE(machine)->mmio_exec) {
>> + memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
>> + &fl->mmio, 0, fl->size);
>> + memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>> + boot_rom);
>> + } else {
>> + memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
>> + fl->size, &error_abort);
>> + memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>> + boot_rom);
>> + write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
>> + }
>
> Nitpick: I'd use rom_add_file_mr() in write_boot_rom(), and keep the memory_region_add_subregion() call after the if/else statement.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
I would rather address changes in follow-up patches.
Thanks for the hint.
C.
>
>> }
>> aspeed_board_binfo.ram_size = ram_size;
>> @@ -398,6 +405,30 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
>> /* Bus 11: TODO ucd90160@64 */
>> }
>> +static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>> +{
>> + return ASPEED_MACHINE(obj)->mmio_exec;
>> +}
>> +
>> +static void aspeed_set_mmio_exec(Object *obj, bool value, Error **errp)
>> +{
>> + ASPEED_MACHINE(obj)->mmio_exec = value;
>> +}
>> +
>> +static void aspeed_machine_instance_init(Object *obj)
>> +{
>> + ASPEED_MACHINE(obj)->mmio_exec = false;
>> +}
>> +
>> +static void aspeed_machine_class_props_init(ObjectClass *oc)
>> +{
>> + object_class_property_add_bool(oc, "execute-in-place",
>> + aspeed_get_mmio_exec,
>> + aspeed_set_mmio_exec, &error_abort);
>> + object_class_property_set_description(oc, "execute-in-place",
>> + "boot directly from CE0 flash device", &error_abort);
>> +}
>> +
>> static void aspeed_machine_class_init(ObjectClass *oc, void *data)
>> {
>> MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -407,6 +438,8 @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data)
>> mc->no_floppy = 1;
>> mc->no_cdrom = 1;
>> mc->no_parallel = 1;
>> +
>> + aspeed_machine_class_props_init(oc);
>> }
>> static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
>> @@ -549,6 +582,7 @@ static const TypeInfo aspeed_machine_types[] = {
>> .name = TYPE_ASPEED_MACHINE,
>> .parent = TYPE_MACHINE,
>> .instance_size = sizeof(AspeedMachine),
>> + .instance_init = aspeed_machine_instance_init,
>> .class_size = sizeof(AspeedMachineClass),
>> .class_init = aspeed_machine_class_init,
>> .abstract = true,
>>
>
© 2016 - 2026 Red Hat, Inc.