Re: [PATCH v2 01/20] hw/arm: aspeed: Store boot info in the machine state

On 16/8/26 15:12, Bin Meng wrote:
> arm_load_kernel() keeps a pointer to the boot info struct for the
> lifetime of the VM, so the struct logically belongs to the machine
> rather than to a file scoped static object.
> 
> Move the boot info into the existing AspeedMachineState.
> 
> As in the xlnx-zcu102 and raspi machines, the boot info belongs to
> the machine rather than to a static object:
> 
> 4d1ac883a7 ("hw/arm: xlnx-zcu102: Move arm_boot_info into XlnxZCU102")
> 0f15c6e338 ("hw/arm/raspi: Move arm_boot_info structure to RaspiMachineState")
> 
> Signed-off-by: Bin Meng <[email protected]>
> ---
> 
>   hw/arm/aspeed.c         | 17 +++++++----------
>   include/hw/arm/aspeed.h |  2 ++
>   2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index a9238e6217..8fa16b3831 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -24,10 +24,6 @@
>   #include "hw/core/qdev-clock.h"
>   #include "system/system.h"
>   
> -static struct arm_boot_info aspeed_board_binfo = {
> -    .board_id = -1, /* device-tree-only board */
> -};


> -    aspeed_board_binfo.ram_size = machine->ram_size;
> -    aspeed_board_binfo.loader_start = sc->memmap[ASPEED_DEV_SDRAM];
> +    bmc->bootinfo.board_id = -1; /* device-tree-only board */

Should #define a self-explanatory BOARD_ID_DEVICE_TREE?

> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index a00238ed74..54f0d7643d 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -12,6 +12,7 @@
>   #include "hw/core/boards.h"
>   #include "qom/object.h"
>   #include "hw/arm/aspeed_soc.h"
> +#include "hw/arm/boot.h"
>   
>   typedef struct AspeedMachineState AspeedMachineState;
>   
> @@ -42,6 +43,7 @@ struct AspeedMachineState {
>       char *fmc_model;
>       char *spi_model;
>       uint32_t hw_strap1;
> +    struct arm_boot_info bootinfo;
>   };

Maybe time to add a typedef to be style-consistent.

Anyhow:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>