jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

arm/boot: split load_dtb() from arm_load_kernel()

load_dtb() depends on arm_load_kernel() to figure out place
in RAM where it should be loaded, but it's not required for
arm_load_kernel() to work. Sometimes it's neccesary for
devices added with -device/device_add to be enumerated in
DTB as well, which's lead to [1] and surrounding commits to
add 2 more machine_done notifiers with non obvious ordering
to make dynamic sysbus devices initialization happen in
the right order.

However instead of moving whole arm_load_kernel() in to
machine_done, it's sufficient to move only load_dtb() into
virt_machine_done() notifier and remove ArmLoadKernelNotifier/
/PlatformBusFDTNotifierParams notifiers, which saves us ~90LOC
and simplifies code flow quite a bit.
Later would allow to consolidate DTB generation within one
function for 'mach-virt' board and make it reentrant so it
could generate updated DTB in device hotplug secenarios.

While at it rename load_dtb() to arm_load_dtb() since it's
public now.

Add additional field skip_dtb_autoload to struct arm_boot_info
to allow manual DTB load later in mach-virt and to avoid touching
all other boards to explicitly call arm_load_dtb().

1) (ac9d32e hw/arm/boot: arm_load_kernel implemented as a machine init done notifier)

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 1525691524-32265-4-git-send-email-imammedo@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

authored by

Igor Mammedov and committed by
Peter Maydell 3b77f6c3 a3fc8396

+94 -185
+19 -53
hw/arm/boot.c
··· 36 36 #define ARM64_TEXT_OFFSET_OFFSET 8 37 37 #define ARM64_MAGIC_OFFSET 56 38 38 39 - static AddressSpace *arm_boot_address_space(ARMCPU *cpu, 40 - const struct arm_boot_info *info) 39 + AddressSpace *arm_boot_address_space(ARMCPU *cpu, 40 + const struct arm_boot_info *info) 41 41 { 42 42 /* Return the address space to use for bootloader reads and writes. 43 43 * We prefer the secure address space if the CPU has it and we're ··· 486 486 qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn); 487 487 } 488 488 489 - /** 490 - * load_dtb() - load a device tree binary image into memory 491 - * @addr: the address to load the image at 492 - * @binfo: struct describing the boot environment 493 - * @addr_limit: upper limit of the available memory area at @addr 494 - * @as: address space to load image to 495 - * 496 - * Load a device tree supplied by the machine or by the user with the 497 - * '-dtb' command line option, and put it at offset @addr in target 498 - * memory. 499 - * 500 - * If @addr_limit contains a meaningful value (i.e., it is strictly greater 501 - * than @addr), the device tree is only loaded if its size does not exceed 502 - * the limit. 503 - * 504 - * Returns: the size of the device tree image on success, 505 - * 0 if the image size exceeds the limit, 506 - * -1 on errors. 507 - * 508 - * Note: Must not be called unless have_dtb(binfo) is true. 509 - */ 510 - static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo, 511 - hwaddr addr_limit, AddressSpace *as) 489 + int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo, 490 + hwaddr addr_limit, AddressSpace *as) 512 491 { 513 492 void *fdt = NULL; 514 493 int size, rc; ··· 935 914 return size; 936 915 } 937 916 938 - static void arm_load_kernel_notify(Notifier *notifier, void *data) 917 + void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) 939 918 { 940 919 CPUState *cs; 941 920 int kernel_size; ··· 945 924 int elf_machine; 946 925 hwaddr entry; 947 926 static const ARMInsnFixup *primary_loader; 948 - ArmLoadKernelNotifier *n = DO_UPCAST(ArmLoadKernelNotifier, 949 - notifier, notifier); 950 - ARMCPU *cpu = n->cpu; 951 - struct arm_boot_info *info = 952 - container_of(n, struct arm_boot_info, load_kernel_notifier); 953 927 AddressSpace *as = arm_boot_address_space(cpu, info); 954 928 955 929 /* The board code is not supposed to set secure_board_setup unless ··· 959 933 assert(!(info->secure_board_setup && kvm_enabled())); 960 934 961 935 info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb"); 936 + info->dtb_limit = 0; 962 937 963 938 /* Load the kernel. */ 964 939 if (!info->kernel_filename || info->firmware_loaded) { ··· 968 943 * the kernel is supposed to be loaded by the bootloader), copy the 969 944 * DTB to the base of RAM for the bootloader to pick up. 970 945 */ 971 - if (load_dtb(info->loader_start, info, 0, as) < 0) { 972 - exit(1); 973 - } 946 + info->dtb_start = info->loader_start; 974 947 } 975 948 976 949 if (info->kernel_filename) { ··· 1050 1023 */ 1051 1024 if (elf_low_addr > info->loader_start 1052 1025 || elf_high_addr < info->loader_start) { 1053 - /* Pass elf_low_addr as address limit to load_dtb if it may be 1026 + /* Set elf_low_addr as address limit for arm_load_dtb if it may be 1054 1027 * pointing into RAM, otherwise pass '0' (no limit) 1055 1028 */ 1056 1029 if (elf_low_addr < info->loader_start) { 1057 1030 elf_low_addr = 0; 1058 1031 } 1059 - if (load_dtb(info->loader_start, info, elf_low_addr, as) < 0) { 1060 - exit(1); 1061 - } 1032 + info->dtb_start = info->loader_start; 1033 + info->dtb_limit = elf_low_addr; 1062 1034 } 1063 1035 } 1064 1036 entry = elf_entry; ··· 1116 1088 */ 1117 1089 if (have_dtb(info)) { 1118 1090 hwaddr align; 1119 - hwaddr dtb_start; 1120 1091 1121 1092 if (elf_machine == EM_AARCH64) { 1122 1093 /* ··· 1136 1107 } 1137 1108 1138 1109 /* Place the DTB after the initrd in memory with alignment. */ 1139 - dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size, align); 1140 - if (load_dtb(dtb_start, info, 0, as) < 0) { 1141 - exit(1); 1142 - } 1143 - fixupcontext[FIXUP_ARGPTR] = dtb_start; 1110 + info->dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size, 1111 + align); 1112 + fixupcontext[FIXUP_ARGPTR] = info->dtb_start; 1144 1113 } else { 1145 1114 fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR; 1146 1115 if (info->ram_size >= (1ULL << 32)) { ··· 1173 1142 for (cs = first_cpu; cs; cs = CPU_NEXT(cs)) { 1174 1143 ARM_CPU(cs)->env.boot_info = info; 1175 1144 } 1176 - } 1177 - 1178 - void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) 1179 - { 1180 - CPUState *cs; 1181 - 1182 - info->load_kernel_notifier.cpu = cpu; 1183 - info->load_kernel_notifier.notifier.notify = arm_load_kernel_notify; 1184 - qemu_add_machine_init_done_notifier(&info->load_kernel_notifier.notifier); 1185 1145 1186 1146 /* CPU objects (unlike devices) are not automatically reset on system 1187 1147 * reset, so we must always register a handler to do so. If we're ··· 1190 1150 */ 1191 1151 for (cs = first_cpu; cs; cs = CPU_NEXT(cs)) { 1192 1152 qemu_register_reset(do_cpu_reset, ARM_CPU(cs)); 1153 + } 1154 + 1155 + if (!info->skip_dtb_autoload && have_dtb(info)) { 1156 + if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as) < 0) { 1157 + exit(1); 1158 + } 1193 1159 } 1194 1160 } 1195 1161
+4 -57
hw/arm/sysbus-fdt.c
··· 49 49 PlatformBusDevice *pbus; 50 50 } PlatformBusFDTData; 51 51 52 - /* 53 - * struct used when calling the machine init done notifier 54 - * that constructs the fdt nodes of platform bus devices 55 - */ 56 - typedef struct PlatformBusFDTNotifierParams { 57 - Notifier notifier; 58 - ARMPlatformBusFDTParams *fdt_params; 59 - } PlatformBusFDTNotifierParams; 60 - 61 52 /* struct that associates a device type name and a node creation function */ 62 53 typedef struct NodeCreationPair { 63 54 const char *typename; ··· 453 444 exit(1); 454 445 } 455 446 456 - /** 457 - * add_all_platform_bus_fdt_nodes - create all the platform bus nodes 458 - * 459 - * builds the parent platform bus node and all the nodes of dynamic 460 - * sysbus devices attached to it. 461 - */ 462 - static void add_all_platform_bus_fdt_nodes(ARMPlatformBusFDTParams *fdt_params) 447 + void platform_bus_add_all_fdt_nodes(void *fdt, const char *intc, hwaddr addr, 448 + hwaddr bus_size, int irq_start) 463 449 { 464 450 const char platcomp[] = "qemu,platform\0simple-bus"; 465 451 PlatformBusDevice *pbus; 466 452 DeviceState *dev; 467 453 gchar *node; 468 - uint64_t addr, size; 469 - int irq_start, dtb_size; 470 - struct arm_boot_info *info = fdt_params->binfo; 471 - const ARMPlatformBusSystemParams *params = fdt_params->system_params; 472 - const char *intc = fdt_params->intc; 473 - void *fdt = info->get_dtb(info, &dtb_size); 474 - 475 - /* 476 - * If the user provided a dtb, we assume the dynamic sysbus nodes 477 - * already are integrated there. This corresponds to a use case where 478 - * the dynamic sysbus nodes are complex and their generation is not yet 479 - * supported. In that case the user can take charge of the guest dt 480 - * while qemu takes charge of the qom stuff. 481 - */ 482 - if (info->dtb_filename) { 483 - return; 484 - } 485 454 486 455 assert(fdt); 487 456 488 - node = g_strdup_printf("/platform@%"PRIx64, params->platform_bus_base); 489 - addr = params->platform_bus_base; 490 - size = params->platform_bus_size; 491 - irq_start = params->platform_bus_first_irq; 457 + node = g_strdup_printf("/platform@%"PRIx64, addr); 492 458 493 459 /* Create a /platform node that we can put all devices into */ 494 460 qemu_fdt_add_subnode(fdt, node); ··· 499 465 */ 500 466 qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1); 501 467 qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1); 502 - qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, size); 468 + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, bus_size); 503 469 504 470 qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", intc); 505 471 ··· 518 484 519 485 g_free(node); 520 486 } 521 - 522 - static void platform_bus_fdt_notify(Notifier *notifier, void *data) 523 - { 524 - PlatformBusFDTNotifierParams *p = DO_UPCAST(PlatformBusFDTNotifierParams, 525 - notifier, notifier); 526 - 527 - add_all_platform_bus_fdt_nodes(p->fdt_params); 528 - g_free(p->fdt_params); 529 - g_free(p); 530 - } 531 - 532 - void arm_register_platform_bus_fdt_creator(ARMPlatformBusFDTParams *fdt_params) 533 - { 534 - PlatformBusFDTNotifierParams *p = g_new(PlatformBusFDTNotifierParams, 1); 535 - 536 - p->fdt_params = fdt_params; 537 - p->notifier.notify = platform_bus_fdt_notify; 538 - qemu_add_machine_init_done_notifier(&p->notifier); 539 - }
+30 -34
hw/arm/virt.c
··· 94 94 95 95 #define PLATFORM_BUS_NUM_IRQS 64 96 96 97 - static ARMPlatformBusSystemParams platform_bus_params; 98 - 99 97 /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means 100 98 * RAM can go up to the 256GB mark, leaving 256GB of the physical 101 99 * address space unallocated and free for future use between 256G and 512G. ··· 1126 1124 DeviceState *dev; 1127 1125 SysBusDevice *s; 1128 1126 int i; 1129 - ARMPlatformBusFDTParams *fdt_params = g_new(ARMPlatformBusFDTParams, 1); 1130 1127 MemoryRegion *sysmem = get_system_memory(); 1131 - 1132 - platform_bus_params.platform_bus_base = vms->memmap[VIRT_PLATFORM_BUS].base; 1133 - platform_bus_params.platform_bus_size = vms->memmap[VIRT_PLATFORM_BUS].size; 1134 - platform_bus_params.platform_bus_first_irq = vms->irqmap[VIRT_PLATFORM_BUS]; 1135 - platform_bus_params.platform_bus_num_irqs = PLATFORM_BUS_NUM_IRQS; 1136 - 1137 - fdt_params->system_params = &platform_bus_params; 1138 - fdt_params->binfo = &vms->bootinfo; 1139 - fdt_params->intc = "/intc"; 1140 - /* 1141 - * register a machine init done notifier that creates the device tree 1142 - * nodes of the platform bus and its children dynamic sysbus devices 1143 - */ 1144 - arm_register_platform_bus_fdt_creator(fdt_params); 1145 1128 1146 1129 dev = qdev_create(NULL, TYPE_PLATFORM_BUS_DEVICE); 1147 1130 dev->id = TYPE_PLATFORM_BUS_DEVICE; 1148 - qdev_prop_set_uint32(dev, "num_irqs", 1149 - platform_bus_params.platform_bus_num_irqs); 1150 - qdev_prop_set_uint32(dev, "mmio_size", 1151 - platform_bus_params.platform_bus_size); 1131 + qdev_prop_set_uint32(dev, "num_irqs", PLATFORM_BUS_NUM_IRQS); 1132 + qdev_prop_set_uint32(dev, "mmio_size", vms->memmap[VIRT_PLATFORM_BUS].size); 1152 1133 qdev_init_nofail(dev); 1153 1134 vms->platform_bus_dev = dev; 1154 - s = SYS_BUS_DEVICE(dev); 1155 1135 1156 - for (i = 0; i < platform_bus_params.platform_bus_num_irqs; i++) { 1157 - int irqn = platform_bus_params.platform_bus_first_irq + i; 1136 + s = SYS_BUS_DEVICE(dev); 1137 + for (i = 0; i < PLATFORM_BUS_NUM_IRQS; i++) { 1138 + int irqn = vms->irqmap[VIRT_PLATFORM_BUS] + i; 1158 1139 sysbus_connect_irq(s, i, pic[irqn]); 1159 1140 } 1160 1141 1161 1142 memory_region_add_subregion(sysmem, 1162 - platform_bus_params.platform_bus_base, 1143 + vms->memmap[VIRT_PLATFORM_BUS].base, 1163 1144 sysbus_mmio_get_region(s, 0)); 1164 1145 } 1165 1146 ··· 1230 1211 { 1231 1212 VirtMachineState *vms = container_of(notifier, VirtMachineState, 1232 1213 machine_done); 1214 + ARMCPU *cpu = ARM_CPU(first_cpu); 1215 + struct arm_boot_info *info = &vms->bootinfo; 1216 + AddressSpace *as = arm_boot_address_space(cpu, info); 1217 + 1218 + /* 1219 + * If the user provided a dtb, we assume the dynamic sysbus nodes 1220 + * already are integrated there. This corresponds to a use case where 1221 + * the dynamic sysbus nodes are complex and their generation is not yet 1222 + * supported. In that case the user can take charge of the guest dt 1223 + * while qemu takes charge of the qom stuff. 1224 + */ 1225 + if (info->dtb_filename == NULL) { 1226 + platform_bus_add_all_fdt_nodes(vms->fdt, "/intc", 1227 + vms->memmap[VIRT_PLATFORM_BUS].base, 1228 + vms->memmap[VIRT_PLATFORM_BUS].size, 1229 + vms->irqmap[VIRT_PLATFORM_BUS]); 1230 + } 1231 + if (arm_load_dtb(info->dtb_start, info, info->dtb_limit, as) < 0) { 1232 + exit(1); 1233 + } 1233 1234 1234 1235 virt_acpi_setup(vms); 1235 1236 virt_build_smbios(vms); ··· 1457 1458 vms->fw_cfg = create_fw_cfg(vms, &address_space_memory); 1458 1459 rom_set_fw(vms->fw_cfg); 1459 1460 1460 - vms->machine_done.notify = virt_machine_done; 1461 - qemu_add_machine_init_done_notifier(&vms->machine_done); 1461 + create_platform_bus(vms, pic); 1462 1462 1463 1463 vms->bootinfo.ram_size = machine->ram_size; 1464 1464 vms->bootinfo.kernel_filename = machine->kernel_filename; ··· 1468 1468 vms->bootinfo.board_id = -1; 1469 1469 vms->bootinfo.loader_start = vms->memmap[VIRT_MEM].base; 1470 1470 vms->bootinfo.get_dtb = machvirt_dtb; 1471 + vms->bootinfo.skip_dtb_autoload = true; 1471 1472 vms->bootinfo.firmware_loaded = firmware_loaded; 1472 1473 arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo); 1473 1474 1474 - /* 1475 - * arm_load_kernel machine init done notifier registration must 1476 - * happen before the platform_bus_create call. In this latter, 1477 - * another notifier is registered which adds platform bus nodes. 1478 - * Notifiers are executed in registration reverse order. 1479 - */ 1480 - create_platform_bus(vms, pic); 1475 + vms->machine_done.notify = virt_machine_done; 1476 + qemu_add_machine_init_done_notifier(&vms->machine_done); 1481 1477 } 1482 1478 1483 1479 static bool virt_get_secure(Object *obj, Error **errp)
+34 -11
include/hw/arm/arm.h
··· 39 39 */ 40 40 void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size); 41 41 42 - /* 43 - * struct used as a parameter of the arm_load_kernel machine init 44 - * done notifier 45 - */ 46 - typedef struct { 47 - Notifier notifier; /* actual notifier */ 48 - ARMCPU *cpu; /* handle to the first cpu object */ 49 - } ArmLoadKernelNotifier; 50 - 51 42 /* arm_boot.c */ 52 43 struct arm_boot_info { 53 44 uint64_t ram_size; ··· 56 47 const char *initrd_filename; 57 48 const char *dtb_filename; 58 49 hwaddr loader_start; 50 + hwaddr dtb_start; 51 + hwaddr dtb_limit; 52 + /* If set to True, arm_load_kernel() will not load DTB. 53 + * It allows board to load DTB manually later. 54 + * (default: False) 55 + */ 56 + bool skip_dtb_autoload; 59 57 /* multicore boards that use the default secondary core boot functions 60 58 * need to put the address of the secondary boot code, the boot reg, 61 59 * and the GIC address in the next 3 values, respectively. boards that ··· 94 92 * the user it should implement this hook. 95 93 */ 96 94 void (*modify_dtb)(const struct arm_boot_info *info, void *fdt); 97 - /* machine init done notifier executing arm_load_dtb */ 98 - ArmLoadKernelNotifier load_kernel_notifier; 99 95 /* Used internally by arm_boot.c */ 100 96 int is_linux; 101 97 hwaddr initrd_start; ··· 142 138 * machine init done notifiers are called in registration reverse order. 143 139 */ 144 140 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info); 141 + 142 + AddressSpace *arm_boot_address_space(ARMCPU *cpu, 143 + const struct arm_boot_info *info); 144 + 145 + /** 146 + * arm_load_dtb() - load a device tree binary image into memory 147 + * @addr: the address to load the image at 148 + * @binfo: struct describing the boot environment 149 + * @addr_limit: upper limit of the available memory area at @addr 150 + * @as: address space to load image to 151 + * 152 + * Load a device tree supplied by the machine or by the user with the 153 + * '-dtb' command line option, and put it at offset @addr in target 154 + * memory. 155 + * 156 + * If @addr_limit contains a meaningful value (i.e., it is strictly greater 157 + * than @addr), the device tree is only loaded if its size does not exceed 158 + * the limit. 159 + * 160 + * Returns: the size of the device tree image on success, 161 + * 0 if the image size exceeds the limit, 162 + * -1 on errors. 163 + * 164 + * Note: Must not be called unless have_dtb(binfo) is true. 165 + */ 166 + int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo, 167 + hwaddr addr_limit, AddressSpace *as); 145 168 146 169 /* Write a secure board setup routine with a dummy handler for SMCs */ 147 170 void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
+7 -30
include/hw/arm/sysbus-fdt.h
··· 24 24 #ifndef HW_ARM_SYSBUS_FDT_H 25 25 #define HW_ARM_SYSBUS_FDT_H 26 26 27 - #include "hw/arm/arm.h" 28 - #include "qemu-common.h" 29 - #include "hw/sysbus.h" 30 - 31 - /* 32 - * struct that contains dimensioning parameters of the platform bus 33 - */ 34 - typedef struct { 35 - hwaddr platform_bus_base; /* start address of the bus */ 36 - hwaddr platform_bus_size; /* size of the bus */ 37 - int platform_bus_first_irq; /* first hwirq assigned to the bus */ 38 - int platform_bus_num_irqs; /* number of hwirq assigned to the bus */ 39 - } ARMPlatformBusSystemParams; 40 - 41 - /* 42 - * struct that contains all relevant info to build the fdt nodes of 43 - * platform bus and attached dynamic sysbus devices 44 - * in the future might be augmented with additional info 45 - * such as PHY, CLK handles ... 46 - */ 47 - typedef struct { 48 - const ARMPlatformBusSystemParams *system_params; 49 - struct arm_boot_info *binfo; 50 - const char *intc; /* parent interrupt controller name */ 51 - } ARMPlatformBusFDTParams; 27 + #include "exec/hwaddr.h" 52 28 53 29 /** 54 - * arm_register_platform_bus_fdt_creator - register a machine init done 55 - * notifier that creates the device tree nodes of the platform bus and 56 - * associated dynamic sysbus devices 30 + * platform_bus_add_all_fdt_nodes - create all the platform bus nodes 31 + * 32 + * builds the parent platform bus node and all the nodes of dynamic 33 + * sysbus devices attached to it. 57 34 */ 58 - void arm_register_platform_bus_fdt_creator(ARMPlatformBusFDTParams *fdt_params); 59 - 35 + void platform_bus_add_all_fdt_nodes(void *fdt, const char *intc, hwaddr addr, 36 + hwaddr bus_size, int irq_start); 60 37 #endif