mbox series

[v8,00/13] bootstd: Convert rockchip and add various fixes and tweaks

Message ID 20230407103651.2588969-1-sjg@chromium.org
Headers show
Series bootstd: Convert rockchip and add various fixes and tweaks | expand

Message

Simon Glass April 7, 2023, 10:36 a.m. UTC
This series converts rockchip boards over to use standard boot. It also
fixes various problems which have come up recently, showing differences
between the current implementation and the distroboot scripts.

This should get us closer to being able to turn down the scripts.

Changes in v8:
- Add new patch to adjust code ordering to work around compiler quirk
- Add new patch to use blk uclass device numbers to set efi bootdev
- Add cover letter

Changes in v7:
- Fix BOOT_DEFAULT typo in commit message
- Don't resync after defconfig changes

Changes in v6:
- Fix 'unable' typo
- Add new patch to report missing labels only when asked
- Add new patch to show a message sometimes if no bootflows are found
- Add new patch to disable BOOTSTD for boards with custom commands
- Add new patch to disable CONFIG_BOOTSTD_DEFAULTS for some xilinx boards
- Add new patch to disable SPL_EFI_PARTITION for sama5d27 board
- Redo patch for the new approach

Changes in v5:
- Add new patch to tweak bootflow logic for device tree
- Add new patch to ensure PCI is set up first when using virtio
- Add new patch to support booting EFI where multiple options exist
- Drop patch to relax the argument requirements for bootflow scan

Changes in v4:
- Add back BOOT_TARGETS
- Rebase to -next
- Add new patch to use the same boot_targets for all boards

Changes in v3:
- Update rk3588 boards too

Changes in v2:
- Add new patch to move rockchip to standard boot

Mathew McBride (1):
  bootstd: Use blk uclass device numbers to set efi bootdev

Simon Glass (12):
  bootstd: Tweak bootflow logic for device tree
  virtio: Ensure PCI is set up first
  bootstd: Support booting EFI where multiple options exist
  bootstd: Report missing labels only when asked
  bootstd: Show a message sometimes if no bootflows are found
  bootstd: Adjust code ordering to work around compiler quirk
  rockchip: Move to standard boot
  rockchip: Use the same boot_targets for all boards
  bootstd: Disable BOOTSTD for boards with custom commands
  xilinx: Disable CONFIG_BOOTSTD_DEFAULTS for some xilinx boards
  atmel: Disable SPL_EFI_PARTITION for sama5d27_som1_ek_qspiflash
  bootstd: Enable BOOTSTD_DEFAULTS by default

 boot/Kconfig                                  |  1 +
 boot/bootdev-uclass.c                         | 37 ++++++++---
 boot/bootmeth_efi.c                           | 49 +++++++++------
 cmd/bootflow.c                                |  3 +
 configs/P1010RDB-PA_36BIT_NAND_defconfig      |  1 +
 configs/P1010RDB-PA_36BIT_NOR_defconfig       |  1 +
 configs/P1010RDB-PA_36BIT_SDCARD_defconfig    |  1 +
 configs/P1010RDB-PA_36BIT_SPIFLASH_defconfig  |  1 +
 configs/P1010RDB-PA_NAND_defconfig            |  1 +
 configs/P1010RDB-PA_NOR_defconfig             |  1 +
 configs/P1010RDB-PA_SDCARD_defconfig          |  1 +
 configs/P1010RDB-PA_SPIFLASH_defconfig        |  1 +
 configs/P1010RDB-PB_36BIT_NAND_defconfig      |  1 +
 configs/P1010RDB-PB_36BIT_NOR_defconfig       |  1 +
 configs/P1010RDB-PB_36BIT_SDCARD_defconfig    |  1 +
 configs/P1010RDB-PB_36BIT_SPIFLASH_defconfig  |  1 +
 configs/P1010RDB-PB_NAND_defconfig            |  1 +
 configs/P1010RDB-PB_NOR_defconfig             |  1 +
 configs/P1010RDB-PB_SDCARD_defconfig          |  1 +
 configs/P1010RDB-PB_SPIFLASH_defconfig        |  1 +
 configs/P1020RDB-PC_36BIT_NAND_defconfig      |  1 +
 configs/P1020RDB-PC_36BIT_SDCARD_defconfig    |  1 +
 configs/P1020RDB-PC_36BIT_SPIFLASH_defconfig  |  1 +
 configs/P1020RDB-PC_36BIT_defconfig           |  1 +
 configs/P1020RDB-PC_NAND_defconfig            |  1 +
 configs/P1020RDB-PC_SDCARD_defconfig          |  1 +
 configs/P1020RDB-PC_SPIFLASH_defconfig        |  1 +
 configs/P1020RDB-PC_defconfig                 |  1 +
 configs/P1020RDB-PD_NAND_defconfig            |  1 +
 configs/P1020RDB-PD_SDCARD_defconfig          |  1 +
 configs/P1020RDB-PD_SPIFLASH_defconfig        |  1 +
 configs/P1020RDB-PD_defconfig                 |  1 +
 configs/P2020RDB-PC_36BIT_NAND_defconfig      |  1 +
 configs/P2020RDB-PC_36BIT_SDCARD_defconfig    |  1 +
 configs/P2020RDB-PC_36BIT_SPIFLASH_defconfig  |  1 +
 configs/P2020RDB-PC_36BIT_defconfig           |  1 +
 configs/P2020RDB-PC_NAND_defconfig            |  1 +
 configs/P2020RDB-PC_SDCARD_defconfig          |  1 +
 configs/P2020RDB-PC_SPIFLASH_defconfig        |  1 +
 configs/P2020RDB-PC_defconfig                 |  1 +
 configs/P2041RDB_NAND_defconfig               |  1 +
 configs/P2041RDB_SDCARD_defconfig             |  1 +
 configs/P2041RDB_SPIFLASH_defconfig           |  1 +
 configs/P2041RDB_defconfig                    |  1 +
 configs/T1024RDB_NAND_defconfig               |  1 +
 configs/T1024RDB_SDCARD_defconfig             |  1 +
 configs/T1024RDB_SPIFLASH_defconfig           |  1 +
 configs/T1024RDB_defconfig                    |  1 +
 configs/T1042D4RDB_NAND_defconfig             |  1 +
 configs/T1042D4RDB_SDCARD_defconfig           |  1 +
 configs/T1042D4RDB_SPIFLASH_defconfig         |  1 +
 configs/T1042D4RDB_defconfig                  |  1 +
 configs/T2080QDS_NAND_defconfig               |  1 +
 configs/T2080QDS_SDCARD_defconfig             |  1 +
 configs/T2080QDS_SECURE_BOOT_defconfig        |  1 +
 configs/T2080QDS_SPIFLASH_defconfig           |  1 +
 configs/T2080QDS_SRIO_PCIE_BOOT_defconfig     |  1 +
 configs/T2080QDS_defconfig                    |  1 +
 configs/T2080RDB_NAND_defconfig               |  1 +
 configs/T2080RDB_SDCARD_defconfig             |  1 +
 configs/T2080RDB_SPIFLASH_defconfig           |  1 +
 configs/T2080RDB_defconfig                    |  1 +
 configs/T2080RDB_revD_NAND_defconfig          |  1 +
 configs/T2080RDB_revD_SDCARD_defconfig        |  1 +
 configs/T2080RDB_revD_SPIFLASH_defconfig      |  1 +
 configs/T2080RDB_revD_defconfig               |  1 +
 configs/T4240RDB_SDCARD_defconfig             |  1 +
 configs/T4240RDB_defconfig                    |  1 +
 configs/am335x_baltos_defconfig               |  1 +
 configs/am335x_igep003x_defconfig             |  1 +
 configs/am335x_pdu001_defconfig               |  1 +
 configs/am335x_shc_defconfig                  |  1 +
 configs/am335x_shc_ict_defconfig              |  1 +
 configs/am335x_shc_netboot_defconfig          |  1 +
 configs/am335x_shc_sdboot_defconfig           |  1 +
 configs/am3517_evm_defconfig                  |  1 +
 configs/am64x_evm_a53_defconfig               |  1 +
 configs/am65x_hs_evm_a53_defconfig            |  1 +
 configs/arbel_evb_defconfig                   |  1 +
 configs/aristainetos2c_defconfig              |  1 +
 configs/aristainetos2ccslb_defconfig          |  1 +
 configs/at91sam9260ek_dataflash_cs0_defconfig |  1 +
 configs/at91sam9260ek_dataflash_cs1_defconfig |  1 +
 configs/at91sam9260ek_nandflash_defconfig     |  1 +
 configs/at91sam9261ek_dataflash_cs0_defconfig |  1 +
 configs/at91sam9261ek_dataflash_cs3_defconfig |  1 +
 configs/at91sam9261ek_nandflash_defconfig     |  1 +
 configs/at91sam9263ek_dataflash_cs0_defconfig |  1 +
 configs/at91sam9263ek_dataflash_defconfig     |  1 +
 configs/at91sam9263ek_nandflash_defconfig     |  1 +
 configs/at91sam9g10ek_dataflash_cs0_defconfig |  1 +
 configs/at91sam9g10ek_dataflash_cs3_defconfig |  1 +
 configs/at91sam9g10ek_nandflash_defconfig     |  1 +
 configs/at91sam9g20ek_2mmc_defconfig          |  1 +
 .../at91sam9g20ek_2mmc_nandflash_defconfig    |  1 +
 configs/at91sam9g20ek_dataflash_cs0_defconfig |  1 +
 configs/at91sam9g20ek_dataflash_cs1_defconfig |  1 +
 configs/at91sam9g20ek_nandflash_defconfig     |  1 +
 configs/at91sam9m10g45ek_mmc_defconfig        |  1 +
 configs/at91sam9m10g45ek_nandflash_defconfig  |  1 +
 configs/at91sam9n12ek_mmc_defconfig           |  1 +
 configs/at91sam9n12ek_nandflash_defconfig     |  1 +
 configs/at91sam9n12ek_spiflash_defconfig      |  1 +
 configs/at91sam9rlek_dataflash_defconfig      |  1 +
 configs/at91sam9rlek_mmc_defconfig            |  1 +
 configs/at91sam9rlek_nandflash_defconfig      |  1 +
 configs/at91sam9x5ek_dataflash_defconfig      |  1 +
 configs/at91sam9x5ek_nandflash_defconfig      |  1 +
 configs/at91sam9x5ek_spiflash_defconfig       |  1 +
 configs/at91sam9xeek_dataflash_cs0_defconfig  |  1 +
 configs/at91sam9xeek_dataflash_cs1_defconfig  |  1 +
 configs/at91sam9xeek_nandflash_defconfig      |  1 +
 configs/bayleybay_defconfig                   |  1 +
 configs/bcm_ns3_defconfig                     |  1 +
 configs/bk4r1_defconfig                       |  1 +
 configs/brppt1_mmc_defconfig                  |  1 +
 configs/brppt2_defconfig                      |  1 +
 configs/brsmarc1_defconfig                    |  1 +
 configs/brxre1_defconfig                      |  1 +
 configs/cgtqmx8_defconfig                     |  1 +
 configs/cherryhill_defconfig                  |  1 +
 configs/chiliboard_defconfig                  |  1 +
 configs/chromebook_coral_defconfig            |  1 +
 configs/chromebook_link64_defconfig           |  1 +
 configs/chromebook_link_defconfig             |  1 +
 configs/chromebook_samus_defconfig            |  1 +
 configs/chromebook_samus_tpl_defconfig        |  1 +
 configs/chromebox_panther_defconfig           |  1 +
 configs/ci20_mmc_defconfig                    |  1 +
 configs/cl-som-imx7_defconfig                 |  1 +
 configs/cm_t43_defconfig                      |  1 +
 ...-qeval20-qa3-e3845-internal-uart_defconfig |  1 +
 configs/conga-qeval20-qa3-e3845_defconfig     |  1 +
 configs/controlcenterdc_defconfig             |  1 +
 configs/coreboot64_defconfig                  |  1 +
 configs/coreboot_defconfig                    |  1 +
 configs/corvus_defconfig                      |  1 +
 configs/cougarcanyon2_defconfig               |  1 +
 configs/crownbay_defconfig                    |  1 +
 configs/d2net_v2_defconfig                    |  1 +
 configs/da850evm_defconfig                    |  1 +
 configs/da850evm_direct_nor_defconfig         |  1 +
 configs/da850evm_nand_defconfig               |  1 +
 configs/deneb_defconfig                       |  1 +
 configs/devkit8000_defconfig                  |  1 +
 configs/dfi-bt700-q7x-151_defconfig           |  1 +
 configs/display5_defconfig                    |  1 +
 configs/display5_factory_defconfig            |  1 +
 configs/dns325_defconfig                      |  1 +
 configs/dockstar_defconfig                    |  1 +
 configs/dreamplug_defconfig                   |  1 +
 configs/ds109_defconfig                       |  1 +
 configs/ds414_defconfig                       |  1 +
 configs/efi-x86_payload32_defconfig           |  1 +
 configs/efi-x86_payload64_defconfig           |  1 +
 configs/ethernut5_defconfig                   |  1 +
 configs/evb-ast2600_defconfig                 |  1 +
 configs/evb-rv1108_defconfig                  |  1 +
 configs/galileo_defconfig                     |  1 +
 configs/gazerbeam_defconfig                   |  1 +
 configs/ge_b1x5v2_defconfig                   |  1 +
 configs/ge_bx50v3_defconfig                   |  1 +
 configs/giedi_defconfig                       |  1 +
 configs/goflexhome_defconfig                  |  1 +
 configs/guruplug_defconfig                    |  1 +
 configs/gwventana_emmc_defconfig              |  1 +
 configs/gwventana_nand_defconfig              |  1 +
 configs/hihope_rzg2_defconfig                 |  1 +
 configs/ib62x0_defconfig                      |  1 +
 configs/iconnect_defconfig                    |  1 +
 configs/imx28_xea_defconfig                   |  1 +
 configs/imx6dl_icore_nand_defconfig           |  1 +
 configs/imx6q_bosch_acc_defconfig             |  1 +
 configs/imx6q_icore_nand_defconfig            |  1 +
 configs/imx6q_logic_defconfig                 |  1 +
 configs/imx6qdl_icore_mipi_defconfig          |  1 +
 configs/imx6qdl_icore_mmc_defconfig           |  1 +
 configs/imx6qdl_icore_nand_defconfig          |  1 +
 configs/imx6qdl_icore_rqs_defconfig           |  1 +
 configs/imx6ul_geam_mmc_defconfig             |  1 +
 configs/imx6ul_geam_nand_defconfig            |  1 +
 configs/imx6ul_isiot_emmc_defconfig           |  1 +
 configs/imx6ul_isiot_nand_defconfig           |  1 +
 configs/imx7_cm_defconfig                     |  1 +
 configs/imx8mm-mx8menlo_defconfig             |  1 +
 configs/imx8mm_beacon_defconfig               |  1 +
 configs/imx8mm_data_modul_edm_sbc_defconfig   |  1 +
 configs/imx8mn_beacon_2g_defconfig            |  1 +
 configs/imx8mn_beacon_defconfig               |  1 +
 configs/imx8mn_beacon_fspi_defconfig          |  1 +
 configs/imx8mq_phanbell_defconfig             |  1 +
 configs/imx8qm_mek_defconfig                  |  1 +
 configs/imx8qm_rom7720_a1_4G_defconfig        |  1 +
 configs/imx8qxp_mek_defconfig                 |  1 +
 configs/inetspace_v2_defconfig                |  1 +
 configs/j7200_evm_a72_defconfig               |  1 +
 configs/j7200_hs_evm_a72_defconfig            |  1 +
 configs/j721e_hs_evm_a72_defconfig            |  1 +
 configs/j721s2_evm_a72_defconfig              |  1 +
 configs/j721s2_hs_evm_a72_defconfig           |  1 +
 configs/k2e_evm_defconfig                     |  1 +
 configs/k2e_hs_evm_defconfig                  |  1 +
 configs/k2g_evm_defconfig                     |  1 +
 configs/k2g_hs_evm_defconfig                  |  1 +
 configs/k2hk_evm_defconfig                    |  1 +
 configs/k2hk_hs_evm_defconfig                 |  1 +
 configs/k2l_evm_defconfig                     |  1 +
 configs/k2l_hs_evm_defconfig                  |  1 +
 configs/legoev3_defconfig                     |  1 +
 configs/liteboard_defconfig                   |  1 +
 configs/ls1088aqds_defconfig                  |  1 +
 configs/ls1088aqds_qspi_SECURE_BOOT_defconfig |  1 +
 configs/ls1088aqds_qspi_defconfig             |  1 +
 configs/ls1088aqds_sdcard_ifc_defconfig       |  1 +
 configs/ls1088aqds_sdcard_qspi_defconfig      |  1 +
 configs/ls2080aqds_SECURE_BOOT_defconfig      |  1 +
 configs/ls2080aqds_defconfig                  |  1 +
 configs/ls2080aqds_nand_defconfig             |  1 +
 configs/ls2080aqds_qspi_defconfig             |  1 +
 configs/ls2080aqds_sdcard_defconfig           |  1 +
 configs/m53menlo_defconfig                    |  1 +
 configs/minnowmax_defconfig                   |  1 +
 configs/mx23_olinuxino_defconfig              |  1 +
 configs/mx23evk_defconfig                     |  1 +
 configs/mx28evk_defconfig                     |  1 +
 configs/mx51evk_defconfig                     |  1 +
 configs/mx53loco_defconfig                    |  1 +
 configs/mx53ppd_defconfig                     |  1 +
 configs/mx6sabreauto_defconfig                |  1 +
 configs/mx6sabresd_defconfig                  |  1 +
 configs/mx6slevk_defconfig                    |  1 +
 configs/mx6slevk_spinor_defconfig             |  1 +
 configs/mx6slevk_spl_defconfig                |  1 +
 configs/mx6sllevk_defconfig                   |  1 +
 configs/mx6sllevk_plugin_defconfig            |  1 +
 configs/mx6sxsabreauto_defconfig              |  1 +
 configs/mx6sxsabresd_defconfig                |  1 +
 configs/mx6ul_14x14_evk_defconfig             |  1 +
 configs/mx6ul_9x9_evk_defconfig               |  1 +
 configs/mx6ull_14x14_evk_defconfig            |  1 +
 configs/mx6ull_14x14_evk_plugin_defconfig     |  1 +
 configs/mx6ulz_14x14_evk_defconfig            |  1 +
 configs/mx7ulp_com_defconfig                  |  1 +
 configs/mx7ulp_evk_defconfig                  |  1 +
 configs/mx7ulp_evk_plugin_defconfig           |  1 +
 configs/net2big_v2_defconfig                  |  1 +
 configs/netspace_lite_v2_defconfig            |  1 +
 configs/netspace_max_v2_defconfig             |  1 +
 configs/netspace_mini_v2_defconfig            |  1 +
 configs/netspace_v2_defconfig                 |  1 +
 configs/omap35_logic_defconfig                |  1 +
 configs/omap35_logic_somlv_defconfig          |  1 +
 configs/omap3_logic_defconfig                 |  1 +
 configs/omap3_logic_somlv_defconfig           |  1 +
 configs/omapl138_lcdk_defconfig               |  1 +
 configs/openpiton_riscv64_defconfig           |  1 +
 configs/openpiton_riscv64_spl_defconfig       |  1 +
 configs/openrd_base_defconfig                 |  1 +
 configs/openrd_client_defconfig               |  1 +
 configs/openrd_ultimate_defconfig             |  1 +
 configs/opos6uldev_defconfig                  |  1 +
 configs/origen_defconfig                      |  1 +
 configs/pcm052_defconfig                      |  1 +
 configs/pcm058_defconfig                      |  1 +
 configs/phycore-imx8mm_defconfig              |  1 +
 configs/phycore-imx8mp_defconfig              |  1 +
 configs/phycore_pcl063_ull_defconfig          |  1 +
 configs/pico-imx6_defconfig                   |  1 +
 configs/pico-imx8mq_defconfig                 |  1 +
 configs/pm9261_defconfig                      |  1 +
 configs/pm9263_defconfig                      |  1 +
 configs/pm9g45_defconfig                      |  1 +
 configs/pogo_e02_defconfig                    |  1 +
 configs/poleg_evb_defconfig                   |  1 +
 configs/qemu-ppce500_defconfig                |  1 +
 configs/r8a77980_condor_defconfig             |  1 +
 configs/r8a77990_ebisu_defconfig              |  1 +
 configs/r8a77995_draak_defconfig              |  1 +
 configs/r8a779a0_falcon_defconfig             |  1 +
 configs/rcar3_ulcb_defconfig                  |  1 +
 configs/rzg2_beacon_defconfig                 |  1 +
 configs/s5p_goni_defconfig                    |  1 +
 configs/s5pc210_universal_defconfig           |  1 +
 configs/sam9x60_curiosity_mmc1_defconfig      |  1 +
 configs/sam9x60_curiosity_mmc_defconfig       |  1 +
 configs/sam9x60ek_mmc_defconfig               |  1 +
 configs/sam9x60ek_nandflash_defconfig         |  1 +
 configs/sam9x60ek_qspiflash_defconfig         |  1 +
 configs/sama5d27_giantboard_defconfig         |  1 +
 configs/sama5d27_som1_ek_mmc1_defconfig       |  1 +
 configs/sama5d27_som1_ek_mmc_defconfig        |  1 +
 configs/sama5d27_som1_ek_qspiflash_defconfig  |  1 +
 configs/sama5d27_wlsom1_ek_mmc_defconfig      |  1 +
 .../sama5d27_wlsom1_ek_qspiflash_defconfig    |  1 +
 configs/sama5d2_icp_mmc_defconfig             |  1 +
 configs/sama5d2_icp_qspiflash_defconfig       |  1 +
 configs/sama5d2_ptc_ek_mmc_defconfig          |  1 +
 configs/sama5d2_ptc_ek_nandflash_defconfig    |  1 +
 configs/sama5d2_xplained_emmc_defconfig       |  1 +
 configs/sama5d2_xplained_mmc_defconfig        |  1 +
 configs/sama5d2_xplained_qspiflash_defconfig  |  1 +
 configs/sama5d2_xplained_spiflash_defconfig   |  1 +
 configs/sama5d36ek_cmp_mmc_defconfig          |  1 +
 configs/sama5d36ek_cmp_nandflash_defconfig    |  1 +
 configs/sama5d36ek_cmp_spiflash_defconfig     |  1 +
 configs/sama5d3_xplained_mmc_defconfig        |  1 +
 configs/sama5d3_xplained_nandflash_defconfig  |  1 +
 configs/sama5d3xek_mmc_defconfig              |  1 +
 configs/sama5d3xek_nandflash_defconfig        |  1 +
 configs/sama5d3xek_spiflash_defconfig         |  1 +
 configs/sama5d4_xplained_mmc_defconfig        |  1 +
 configs/sama5d4_xplained_nandflash_defconfig  |  1 +
 configs/sama5d4_xplained_spiflash_defconfig   |  1 +
 configs/sama5d4ek_mmc_defconfig               |  1 +
 configs/sama5d4ek_nandflash_defconfig         |  1 +
 configs/sama5d4ek_spiflash_defconfig          |  1 +
 configs/sama7g5ek_mmc1_defconfig              |  1 +
 configs/sama7g5ek_mmc_defconfig               |  1 +
 configs/sheevaplug_defconfig                  |  1 +
 configs/silinux_ek874_defconfig               |  1 +
 configs/sipeed_maix_bitm_defconfig            |  1 +
 configs/sipeed_maix_smode_defconfig           |  1 +
 configs/slimbootloader_defconfig              |  1 +
 configs/smartweb_defconfig                    |  1 +
 configs/smdkv310_defconfig                    |  1 +
 configs/smegw01_defconfig                     |  1 +
 configs/socfpga_agilex_atf_defconfig          |  1 +
 configs/socfpga_agilex_defconfig              |  1 +
 configs/socfpga_agilex_vab_defconfig          |  1 +
 configs/socfpga_dbm_soc1_defconfig            |  1 +
 configs/socfpga_mcvevk_defconfig              |  1 +
 configs/socfpga_n5x_atf_defconfig             |  1 +
 configs/socfpga_n5x_defconfig                 |  1 +
 configs/socfpga_n5x_vab_defconfig             |  1 +
 configs/socfpga_secu1_defconfig               |  1 +
 configs/socfpga_stratix10_atf_defconfig       |  1 +
 configs/socfpga_stratix10_defconfig           |  1 +
 configs/socfpga_vining_fpga_defconfig         |  1 +
 configs/socrates_defconfig                    |  1 +
 configs/som-db5800-som-6867_defconfig         |  1 +
 configs/somlabs_visionsom_6ull_defconfig      |  1 +
 configs/stemmy_defconfig                      |  1 +
 configs/stm32h750-art-pi_defconfig            |  1 +
 configs/stm32mp13_defconfig                   |  1 +
 ...stm32mp15-icore-stm32mp1-ctouch2_defconfig |  1 +
 ...tm32mp15-icore-stm32mp1-edimm2.2_defconfig |  1 +
 ...-microgea-stm32mp1-microdev2-of7_defconfig |  1 +
 ...mp15-microgea-stm32mp1-microdev2_defconfig |  1 +
 configs/stm32mp15_basic_defconfig             |  1 +
 configs/stm32mp15_defconfig                   |  1 +
 configs/stm32mp15_dhcom_basic_defconfig       |  1 +
 configs/stm32mp15_dhcor_basic_defconfig       |  1 +
 configs/stm32mp15_trusted_defconfig           |  1 +
 configs/taurus_defconfig                      |  1 +
 configs/ti816x_evm_defconfig                  |  1 +
 configs/tools-only_defconfig                  |  2 +-
 configs/topic_miami_defconfig                 |  1 +
 configs/topic_miamilite_defconfig             |  1 +
 configs/topic_miamiplus_defconfig             |  1 +
 configs/total_compute_defconfig               |  1 +
 configs/tplink_wdr4300_defconfig              |  1 +
 configs/tqma6dl_mba6_mmc_defconfig            |  1 +
 configs/tqma6dl_mba6_spi_defconfig            |  1 +
 configs/tqma6q_mba6_mmc_defconfig             |  1 +
 configs/tqma6q_mba6_spi_defconfig             |  1 +
 configs/tqma6s_mba6_mmc_defconfig             |  1 +
 configs/tqma6s_mba6_spi_defconfig             |  1 +
 configs/trats2_defconfig                      |  1 +
 configs/trats_defconfig                       |  1 +
 configs/uniphier_ld4_sld8_defconfig           |  1 +
 configs/uniphier_v7_defconfig                 |  1 +
 configs/uniphier_v8_defconfig                 |  1 +
 configs/variscite_dart6ul_defconfig           |  1 +
 configs/vf610twr_defconfig                    |  1 +
 configs/vf610twr_nand_defconfig               |  1 +
 configs/vinco_defconfig                       |  1 +
 configs/warp7_bl33_defconfig                  |  1 +
 configs/warp7_defconfig                       |  1 +
 configs/xilinx_versal_mini_emmc0_defconfig    |  1 +
 configs/xilinx_versal_mini_emmc1_defconfig    |  1 +
 configs/xilinx_zynqmp_mini_emmc0_defconfig    |  1 +
 configs/xilinx_zynqmp_mini_emmc1_defconfig    |  1 +
 drivers/virtio/virtio-uclass.c                |  6 ++
 include/bootdev.h                             |  2 +-
 include/configs/px30_common.h                 |  3 +-
 include/configs/rk3036_common.h               |  4 +-
 include/configs/rk3066_common.h               |  4 +-
 include/configs/rk3128_common.h               |  3 +-
 include/configs/rk3188_common.h               |  4 +-
 include/configs/rk322x_common.h               |  4 +-
 include/configs/rk3288_common.h               |  4 +-
 include/configs/rk3308_common.h               |  3 +-
 include/configs/rk3328_common.h               |  3 +-
 include/configs/rk3368_common.h               |  6 +-
 include/configs/rk3568_common.h               |  5 +-
 include/configs/rk3588_common.h               |  5 +-
 include/configs/rockchip-common.h             | 62 -------------------
 include/configs/rv1108_common.h               |  2 +-
 test/boot/bootdev.c                           | 12 ++--
 399 files changed, 469 insertions(+), 132 deletions(-)

Comments

Tom Rini April 7, 2023, 7:39 p.m. UTC | #1
On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:

> This series converts rockchip boards over to use standard boot. It also
> fixes various problems which have come up recently, showing differences
> between the current implementation and the distroboot scripts.
> 
> This should get us closer to being able to turn down the scripts.

Alright, so I grabbed a few parts of this series to investigate the
points I'm trying to grasp better, and I think this is going the wrong
track. We should start off by dropping "default y" from BOOTSTD, and
then start adding "default y if" for SoCs as we convert them. The end
goal should be that we get to the point where we can "default y if ARM
|| RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
just a few architectures that haven't ended up being converted. But
today, there's too much churn on platforms that aren't making any use of
this. And I don't think this is going to be functionally worse than all
of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
replace that with "imply BOOTSTD" as they get migrated.
Simon Glass April 7, 2023, 7:53 p.m. UTC | #2
Hi Tom,

On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
>
> > This series converts rockchip boards over to use standard boot. It also
> > fixes various problems which have come up recently, showing differences
> > between the current implementation and the distroboot scripts.
> >
> > This should get us closer to being able to turn down the scripts.
>
> Alright, so I grabbed a few parts of this series to investigate the
> points I'm trying to grasp better, and I think this is going the wrong
> track. We should start off by dropping "default y" from BOOTSTD, and
> then start adding "default y if" for SoCs as we convert them. The end
> goal should be that we get to the point where we can "default y if ARM
> || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> just a few architectures that haven't ended up being converted. But
> today, there's too much churn on platforms that aren't making any use of
> this. And I don't think this is going to be functionally worse than all
> of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> replace that with "imply BOOTSTD" as they get migrated.

That would really be a backward step. I'm not sure what to say at this
point. I've put a lot of effort in trying to get this over the line,
but the only way we get feedback is when it is applied.

What churn are you seeing? Do you mean:

disable BOOTSTD for boards with custom commands? You asked for that patch
disabling BOOTSTD_DEFAULTS? You asked for that patch
enable BOOTSTD_DEFAULTS by default? We can drop it if you like

The other patches are bug fixes as noted below. Passing run at

https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/15934

Size trace:

02: bootstd: Tweak bootflow logic for device tree
       arm:     brsmarc1 j7200_evm_r5 j721e_evm_r5 ge_b1x5v2 stv0991
imxrt1020-evk imxrt1050-evk imxrt1170-evk
   aarch64: (for 383/383 boards) all +36.6 text +36.6
       arm: (for 649/649 boards) all +13.8 text +13.8
   riscv32: (for 7/7 boards) all +34.0 text +34.0
   riscv64: (for 14/14 boards) all +20.4 text +20.4
   sandbox: (for 7/7 boards) all +10.3 text +10.3
       x86: (for 29/29 boards) all +20.9 text +20.9

this is a bug fix, really

03: virtio: Ensure PCI is set up first
   aarch64: (for 383/383 boards) all +0.1 text +0.1
       arm: (for 649/649 boards) all +0.0 text +0.0
   powerpc: (for 81/81 boards) all +0.2 text +0.2
   riscv32: (for 7/7 boards) all +6.9 text +6.9
   riscv64: (for 14/14 boards) all +3.9 text +3.9
   sandbox: (for 7/7 boards) all +17.1 text +17.1
       x86: (for 29/29 boards) all +1.1 text +1.1

this makes it match the scripts

04: bootstd: Support booting EFI where multiple options exist
   aarch64: (for 383/383 boards) all +24.0 rodata +0.3 text +23.8
       arm: (for 649/649 boards) all +13.0 rodata +0.1 text +13.0
   riscv32: (for 7/7 boards) all +40.0 rodata -4.0 text +44.0
   riscv64: (for 14/14 boards) all -13.4 rodata -4.6 text -8.9
   sandbox: (for 7/7 boards) all +36.6 text +36.6
       x86: (for 29/29 boards) all +13.4 text +13.4

bug fix

05: bootstd: Report missing labels only when asked
   aarch64: (for 383/383 boards) all +36.9 rodata -9.5 text +46.5
       arc: (for 11/11 boards) all +13.8 rodata -5.1 text +18.9
       arm: (for 649/649 boards) all +30.0 rodata -8.7 text +38.6
      m68k: (for 18/18 boards) all -0.3 rodata -0.5 text +0.2
      mips: (for 45/45 boards) all +42.7 rodata -3.9 text +46.6
   powerpc: (for 81/81 boards) all +43.8 rodata -7.0 text +50.7
   riscv32: (for 7/7 boards) all +48.0 rodata -8.0 text +56.0
   riscv64: (for 14/14 boards) all +48.3 rodata -8.0 text +56.3
   sandbox: (for 7/7 boards) all -253.7 rodata -64.0
spl/u-boot-spl:all -11.4 spl/u-boot-spl:rodata -9.1
spl/u-boot-spl:text -2.3 text -189.7
        sh: (for 1/1 boards) all +24.0 rodata -8.0 text +32.0
       x86: (for 29/29 boards) all +8.6 rodata -8.4 text +17.0

Patch you requested

06: bootstd: Show a message sometimes if no bootflows are found
   aarch64: (for 383/383 boards) all +0.2 rodata +0.1 text +0.1
       arm: (for 649/649 boards) all +0.2 rodata +0.1 text +0.1
   sandbox: (for 7/7 boards) all +57.1 rodata +36.6 text +20.6

Patch you requested

07: rockchip: Move to standard boot
   aarch64: (for 383/383 boards) all -180.1 rodata -180.1
       arm: (for 649/649 boards) all -108.8 rodata -108.8
08: rockchip: Use the same boot_targets for all boards
   aarch64: (for 383/383 boards) all +0.9 rodata +0.9
       arm: (for 649/649 boards) all +0.5 rodata +0.5

Long-standing conversion patches

09: bootstd: Disable BOOTSTD for boards with custom commands
   aarch64: (for 383/383 boards) all -3314.4 bss -3.6 data -224.2
rodata -312.8 text -2773.8
       arm: (for 649/649 boards) all -6150.4 data -276.3 rodata -640.6
text -5233.5
      mips: (for 45/45 boards) all -1090.7 data -30.9 rodata -103.8 text -955.9
   powerpc: (for 81/81 boards) all -21042.0 bss -9.7 data -951.4
rodata -1713.3 text -18367.6
   riscv64: (for 14/14 boards) all -3567.7 bss -2.3 data -373.1 rodata
-355.4 text -2836.9
   sandbox: (for 7/7 boards) all -3051.6 bss -2.3 data -275.4 rodata
-263.0 text -2510.9
       x86: (for 29/29 boards) all -16307.3 data -834.5 rodata -1790.4
text -13682.4
10: xilinx: Disable CONFIG_BOOTSTD_DEFAULTS for some xilinx boards
       arm: (for 649/649 boards) all +0.0 rodata +0.0
      mips: (for 45/45 boards) all +0.4 rodata +0.4
11: atmel: Disable SPL_EFI_PARTITION for sama5d27_som1_ek_qspiflash
   aarch64: (for 383/383 boards) all +0.1 rodata +0.1
       arm: (for 649/649 boards) all -0.0 rodata -0.0
   sandbox: (for 7/7 boards) all -4.6 rodata -4.6
12: bootstd: Enable BOOTSTD_DEFAULTS by default
   aarch64: (for 383/383 boards) all +4123.1 bss +196.6 data +129.9
rodata +725.8 text +3070.7
       arc: (for 11/11 boards) all +16977.8 bss +1438.9 data +375.3
rodata +5161.5 text +10002.2
       arm: (for 649/649 boards) all +3012.0 data +114.8 rodata +793.4
spl/u-boot-spl:all +54.6 spl/u-boot-spl:bss +1.1 spl/u-boot-spl:data
+0.8 spl/u-boot-spl:rodata +17.8 spl/u-boot-spl:text +35.0 text
+2103.8
      m68k: (for 18/18 boards) all +1373.4 bss +124.0 data +119.8
rodata +387.3 text +742.3
      mips: (for 45/45 boards) all +17823.4 data +458.1 rodata +3532.9
spl/u-boot-spl:all +48.2 spl/u-boot-spl:data +3.6
spl/u-boot-spl:rodata +1.0 spl/u-boot-spl:text +43.6 text +13832.4
   powerpc: (for 81/81 boards) all +679.4 bss +57.8 data +30.8 rodata
+91.4 text +499.4
        sh: (for 1/1 boards) all +32363.0 bss +2283.0 data +1740.0
rodata +8804.0 text +19536.0
       x86: (for 29/29 boards) all +1988.5 data +47.4 rodata +646.4 text +1294.6

Regards,
Simon
Tom Rini April 7, 2023, 8:21 p.m. UTC | #3
On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> >
> > > This series converts rockchip boards over to use standard boot. It also
> > > fixes various problems which have come up recently, showing differences
> > > between the current implementation and the distroboot scripts.
> > >
> > > This should get us closer to being able to turn down the scripts.
> >
> > Alright, so I grabbed a few parts of this series to investigate the
> > points I'm trying to grasp better, and I think this is going the wrong
> > track. We should start off by dropping "default y" from BOOTSTD, and
> > then start adding "default y if" for SoCs as we convert them. The end
> > goal should be that we get to the point where we can "default y if ARM
> > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > just a few architectures that haven't ended up being converted. But
> > today, there's too much churn on platforms that aren't making any use of
> > this. And I don't think this is going to be functionally worse than all
> > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > replace that with "imply BOOTSTD" as they get migrated.
> 
> That would really be a backward step. I'm not sure what to say at this
> point. I've put a lot of effort in trying to get this over the line,
> but the only way we get feedback is when it is applied.

Having bootstd enabled and not functional (because boot_targets aren't
set) isn't helping the migration happen. And the hard part of the
migration isn't knowing it's possible, or enabling 1-2 options, it's
testing it and also it really just being 1-2 options.

> What churn are you seeing? Do you mean:
> 
> disable BOOTSTD for boards with custom commands? You asked for that patch
> disabling BOOTSTD_DEFAULTS? You asked for that patch
> enable BOOTSTD_DEFAULTS by default? We can drop it if you like

We need all 3 of those patches because without the 3rd you don't get a
good experience when you do enable bootstd for a platform. The problem
is that unless you have distro_bootcmd today you're getting between 63kB
(a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
growth because bootstd is on and now is the default bootcmd, when before
they had nothing. And probably had board docs saying "now do ... to
boot". And that's largely setting aside the *_r5_* platforms that I know
are just doing something else, and could disable it.

We want to convert everyone doing distro_bootcmd over to this, that's
good. The problem is we don't have a symbol today that means "we want
distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
in this sense).

The wrong direction part of this series is that for platforms that
aren't in the middle of converting we're increasing their size between
somewhat and very very much, and we haven't tested that it'll work. And
yes, there's some automatic guessing logic, which hasn't been tested on
these platforms either, so we don't actually know if going from no
bootcmd (and so drops to prompt) to attempts to autoboot something is an
improvement.
Simon Glass April 7, 2023, 9:35 p.m. UTC | #4
Hi Tom,

On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > >
> > > > This series converts rockchip boards over to use standard boot. It also
> > > > fixes various problems which have come up recently, showing differences
> > > > between the current implementation and the distroboot scripts.
> > > >
> > > > This should get us closer to being able to turn down the scripts.
> > >
> > > Alright, so I grabbed a few parts of this series to investigate the
> > > points I'm trying to grasp better, and I think this is going the wrong
> > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > then start adding "default y if" for SoCs as we convert them. The end
> > > goal should be that we get to the point where we can "default y if ARM
> > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > just a few architectures that haven't ended up being converted. But
> > > today, there's too much churn on platforms that aren't making any use of
> > > this. And I don't think this is going to be functionally worse than all
> > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > replace that with "imply BOOTSTD" as they get migrated.
> >
> > That would really be a backward step. I'm not sure what to say at this
> > point. I've put a lot of effort in trying to get this over the line,
> > but the only way we get feedback is when it is applied.
>
> Having bootstd enabled and not functional (because boot_targets aren't
> set) isn't helping the migration happen. And the hard part of the
> migration isn't knowing it's possible, or enabling 1-2 options, it's
> testing it and also it really just being 1-2 options.

Standard boot does not need boot_targets to be set. It works fine
without it. It just goes through the boot devices in a pre-defined
order, from fastest to slowest. It matches what most boards do anyway.
The main reason we kept it is for compatibility with distro boot.

I don't think testing in advance is a feasible approach in general.
See for example the rpi series which hasn't got any comment. It likely
won't until it is applied. That's how we get feedback. We have months
to resolve issues and I believe that the code is fundamentally sound.

>
> > What churn are you seeing? Do you mean:
> >
> > disable BOOTSTD for boards with custom commands? You asked for that patch
> > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
>
> We need all 3 of those patches because without the 3rd you don't get a
> good experience when you do enable bootstd for a platform. The problem
> is that unless you have distro_bootcmd today you're getting between 63kB
> (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> growth because bootstd is on and now is the default bootcmd, when before
> they had nothing. And probably had board docs saying "now do ... to
> boot". And that's largely setting aside the *_r5_* platforms that I know
> are just doing something else, and could disable it.

Er, I thought you wanted it to default on if the boards has no
bootcmd? If not, we can disable it for those as well. If you don't
want any increase we can disable it for boards without DISTRO_DEFAULTS
too. After all, presumably those boards are doing something custom
anyway.

>
> We want to convert everyone doing distro_bootcmd over to this, that's
> good. The problem is we don't have a symbol today that means "we want
> distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> in this sense).
>
> The wrong direction part of this series is that for platforms that
> aren't in the middle of converting we're increasing their size between
> somewhat and very very much, and we haven't tested that it'll work. And
> yes, there's some automatic guessing logic, which hasn't been tested on
> these platforms either, so we don't actually know if going from no
> bootcmd (and so drops to prompt) to attempts to autoboot something is an
> improvement.

So, the wrong direction comes from the last three patches. Is that right?

Fundamentally the problem I have is that I know where I would like
this to head, which is everything using standard boot and turning down
the scripts. But it feels like every time I touch bootstd we have to
have the EFI discussion again. You can imagine how I feel about
disabling BOOTSTD by default...it would basically kill it.

This is not really an arch-specific thing, nor an SoC-specific thing.
The underlying logic is the same for everything. The reason I think we
need to do a few cases before we enable it everywhere is that we need
to find the little tweaks needed in that logic.

How about we apply the first patches in this series, skipping the last
three, then apply the rpi series as well. That should get people
actually using it and we can iron out the problems. It also keeps
things moving. We have months before the release.

Enabling by default can come later once we decide what we want to do
about size increases, boards that don't use DISTRO_DEFAULTS and boards
that don't have a boot command.

Regards,
Simon
Tom Rini April 7, 2023, 9:55 p.m. UTC | #5
On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > >
> > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > fixes various problems which have come up recently, showing differences
> > > > > between the current implementation and the distroboot scripts.
> > > > >
> > > > > This should get us closer to being able to turn down the scripts.
> > > >
> > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > goal should be that we get to the point where we can "default y if ARM
> > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > just a few architectures that haven't ended up being converted. But
> > > > today, there's too much churn on platforms that aren't making any use of
> > > > this. And I don't think this is going to be functionally worse than all
> > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > replace that with "imply BOOTSTD" as they get migrated.
> > >
> > > That would really be a backward step. I'm not sure what to say at this
> > > point. I've put a lot of effort in trying to get this over the line,
> > > but the only way we get feedback is when it is applied.
> >
> > Having bootstd enabled and not functional (because boot_targets aren't
> > set) isn't helping the migration happen. And the hard part of the
> > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > testing it and also it really just being 1-2 options.
> 
> Standard boot does not need boot_targets to be set. It works fine
> without it. It just goes through the boot devices in a pre-defined
> order, from fastest to slowest. It matches what most boards do anyway.
> The main reason we kept it is for compatibility with distro boot.

What most boards do today is just sit at the prompt and wait for input,
which this changes, which is part of the big source of size churn here.

> I don't think testing in advance is a feasible approach in general.
> See for example the rpi series which hasn't got any comment. It likely
> won't until it is applied. That's how we get feedback. We have months
> to resolve issues and I believe that the code is fundamentally sound.

We need some spot testing here and there to see how things react and how
people use it. You found a lot of things with just rk3399, and now the
rest of rockchip looks to be fairly direct. You found more things doing
x86. When you can convert some other SoC and the change is just dropping
distro_bootcmd and calling bootflow scan instead (or that + setting the
order again), that'll be good.

But I'm not sure that changing the platforms that don't today opt-in to
distro_bootcmd (which has been a thing for a long time now) to force
opt-in to this is the right call. It might be in some cases (mediatek
maybe? Or maybe no, everyone does android of some flavour so a different
bootstd option) but not others (those *_evm_r5_defconfig boards).

> > > What churn are you seeing? Do you mean:
> > >
> > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> >
> > We need all 3 of those patches because without the 3rd you don't get a
> > good experience when you do enable bootstd for a platform. The problem
> > is that unless you have distro_bootcmd today you're getting between 63kB
> > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > growth because bootstd is on and now is the default bootcmd, when before
> > they had nothing. And probably had board docs saying "now do ... to
> > boot". And that's largely setting aside the *_r5_* platforms that I know
> > are just doing something else, and could disable it.
> 
> Er, I thought you wanted it to default on if the boards has no
> bootcmd? If not, we can disable it for those as well. If you don't
> want any increase we can disable it for boards without DISTRO_DEFAULTS
> too. After all, presumably those boards are doing something custom
> anyway.

I think I might have originally, but now that I'm looking at the results
it was too optimistic. Every branch I build I look at the per-board
breakdown, not just the summary. And it's too much on all of these
platforms that had no default bootcmd today.

> > We want to convert everyone doing distro_bootcmd over to this, that's
> > good. The problem is we don't have a symbol today that means "we want
> > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > in this sense).
> >
> > The wrong direction part of this series is that for platforms that
> > aren't in the middle of converting we're increasing their size between
> > somewhat and very very much, and we haven't tested that it'll work. And
> > yes, there's some automatic guessing logic, which hasn't been tested on
> > these platforms either, so we don't actually know if going from no
> > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > improvement.
> 
> So, the wrong direction comes from the last three patches. Is that right?

The wrong direction comes from enabling bootstd (and so, a bootcmd and
distro boot) on platforms that just sit at the prompt today.  We are not
making a good heuristic guess at what the should be doing.  Reaching out
to the maintainers to get them to do the conversion, especially once
it's just a matter for most of them of just enabling bootstd and then
distro, if they want that or once bootmeth android gets done, for those
platforms) is how we get them moved smoothly.

> Fundamentally the problem I have is that I know where I would like
> this to head, which is everything using standard boot and turning down
> the scripts. But it feels like every time I touch bootstd we have to
> have the EFI discussion again. You can imagine how I feel about
> disabling BOOTSTD by default...it would basically kill it.

Well, we enabled bootstd by default too quickly perhaps then, and just
like we narrowed down EFI_LOADER defaults, we need to narrow this down
until it's easily convertable.

> This is not really an arch-specific thing, nor an SoC-specific thing.
> The underlying logic is the same for everything. The reason I think we
> need to do a few cases before we enable it everywhere is that we need
> to find the little tweaks needed in that logic.

It's a generic framework to a board specific thing.

> How about we apply the first patches in this series, skipping the last
> three, then apply the rpi series as well. That should get people
> actually using it and we can iron out the problems. It also keeps
> things moving. We have months before the release.
> 
> Enabling by default can come later once we decide what we want to do
> about size increases, boards that don't use DISTRO_DEFAULTS and boards
> that don't have a boot command.

How about disabling it by default and imply'ing it for everything that
implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
done is the distro bootmeth?
Simon Glass April 8, 2023, 12:08 a.m. UTC | #6
Hi Tom,

On Sat, 8 Apr 2023 at 09:55, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > > >
> > > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > > fixes various problems which have come up recently, showing differences
> > > > > > between the current implementation and the distroboot scripts.
> > > > > >
> > > > > > This should get us closer to being able to turn down the scripts.
> > > > >
> > > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > > goal should be that we get to the point where we can "default y if ARM
> > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > > just a few architectures that haven't ended up being converted. But
> > > > > today, there's too much churn on platforms that aren't making any use of
> > > > > this. And I don't think this is going to be functionally worse than all
> > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > > replace that with "imply BOOTSTD" as they get migrated.
> > > >
> > > > That would really be a backward step. I'm not sure what to say at this
> > > > point. I've put a lot of effort in trying to get this over the line,
> > > > but the only way we get feedback is when it is applied.
> > >
> > > Having bootstd enabled and not functional (because boot_targets aren't
> > > set) isn't helping the migration happen. And the hard part of the
> > > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > > testing it and also it really just being 1-2 options.
> >
> > Standard boot does not need boot_targets to be set. It works fine
> > without it. It just goes through the boot devices in a pre-defined
> > order, from fastest to slowest. It matches what most boards do anyway.
> > The main reason we kept it is for compatibility with distro boot.
>
> What most boards do today is just sit at the prompt and wait for input,
> which this changes, which is part of the big source of size churn here.

Yes.

>
> > I don't think testing in advance is a feasible approach in general.
> > See for example the rpi series which hasn't got any comment. It likely
> > won't until it is applied. That's how we get feedback. We have months
> > to resolve issues and I believe that the code is fundamentally sound.
>
> We need some spot testing here and there to see how things react and how
> people use it. You found a lot of things with just rk3399, and now the
> rest of rockchip looks to be fairly direct. You found more things doing
> x86. When you can convert some other SoC and the change is just dropping
> distro_bootcmd and calling bootflow scan instead (or that + setting the
> order again), that'll be good.

So long as we are aware that we generally only find problems when
patches land, yes. The QEMU stuff is easier since it doesn't need a
board. It's also not all that useful in the real world :-) I'd like to
try a programmatic conversion, too, although I haven't looked at it
yet.

>
> But I'm not sure that changing the platforms that don't today opt-in to
> distro_bootcmd (which has been a thing for a long time now) to force
> opt-in to this is the right call. It might be in some cases (mediatek
> maybe? Or maybe no, everyone does android of some flavour so a different
> bootstd option) but not others (those *_evm_r5_defconfig boards).

Fair enough, so long as we actually turn down distro_bootcmd. So long
as it is still there, boards will enable it. See SPL_FIT_GENERATOR.

>
> > > > What churn are you seeing? Do you mean:
> > > >
> > > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> > >
> > > We need all 3 of those patches because without the 3rd you don't get a
> > > good experience when you do enable bootstd for a platform. The problem
> > > is that unless you have distro_bootcmd today you're getting between 63kB
> > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > > growth because bootstd is on and now is the default bootcmd, when before
> > > they had nothing. And probably had board docs saying "now do ... to
> > > boot". And that's largely setting aside the *_r5_* platforms that I know
> > > are just doing something else, and could disable it.
> >
> > Er, I thought you wanted it to default on if the boards has no
> > bootcmd? If not, we can disable it for those as well. If you don't
> > want any increase we can disable it for boards without DISTRO_DEFAULTS
> > too. After all, presumably those boards are doing something custom
> > anyway.
>
> I think I might have originally, but now that I'm looking at the results
> it was too optimistic. Every branch I build I look at the per-board
> breakdown, not just the summary. And it's too much on all of these
> platforms that had no default bootcmd today.

OK. I don't mind about that. We've ended up in a bit of a rabbit hole I think.

>
> > > We want to convert everyone doing distro_bootcmd over to this, that's
> > > good. The problem is we don't have a symbol today that means "we want
> > > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > > in this sense).
> > >
> > > The wrong direction part of this series is that for platforms that
> > > aren't in the middle of converting we're increasing their size between
> > > somewhat and very very much, and we haven't tested that it'll work. And
> > > yes, there's some automatic guessing logic, which hasn't been tested on
> > > these platforms either, so we don't actually know if going from no
> > > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > > improvement.
> >
> > So, the wrong direction comes from the last three patches. Is that right?
>
> The wrong direction comes from enabling bootstd (and so, a bootcmd and
> distro boot) on platforms that just sit at the prompt today.  We are not
> making a good heuristic guess at what the should be doing.  Reaching out
> to the maintainers to get them to do the conversion, especially once
> it's just a matter for most of them of just enabling bootstd and then
> distro, if they want that or once bootmeth android gets done, for those
> platforms) is how we get them moved smoothly.

We should know which boards sit at the prompt today, by the fact that
they don't have a bootcmd. Presumably that is the way the maintainer
wants it, so I agree we should avoid changing it.

>
> > Fundamentally the problem I have is that I know where I would like
> > this to head, which is everything using standard boot and turning down
> > the scripts. But it feels like every time I touch bootstd we have to
> > have the EFI discussion again. You can imagine how I feel about
> > disabling BOOTSTD by default...it would basically kill it.
>
> Well, we enabled bootstd by default too quickly perhaps then, and just
> like we narrowed down EFI_LOADER defaults, we need to narrow this down
> until it's easily convertable.

It feels like it took a year to get that moving. There was so much EFI
discussion that I really don't want to revisit.

>
> > This is not really an arch-specific thing, nor an SoC-specific thing.
> > The underlying logic is the same for everything. The reason I think we
> > need to do a few cases before we enable it everywhere is that we need
> > to find the little tweaks needed in that logic.
>
> It's a generic framework to a board specific thing.
>
> > How about we apply the first patches in this series, skipping the last
> > three, then apply the rpi series as well. That should get people
> > actually using it and we can iron out the problems. It also keeps
> > things moving. We have months before the release.
> >
> > Enabling by default can come later once we decide what we want to do
> > about size increases, boards that don't use DISTRO_DEFAULTS and boards
> > that don't have a boot command.
>
> How about disabling it by default and imply'ing it for everything that
> implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
> done is the distro bootmeth?

I'd really like to move forwards, instead of creating another barrier
to this migration. There was a huge amount of work in making sure that
the incremental size of bootstd was small, so it could be cheaply
enabled. This all went off the rails because, as you correctly pointed
out, enabling the bootstd commands does ~nothing if there is no actual
boot command set. I wish I had just stopped then to clarify the goals,
because this has all burned a lot of time and energy.

From my side the best thing to do would be to get the currently
outstanding migrations into -master ASAP so people can try them out.
Despite the delays we still have months left for testing on this
release. Then I (or perhaps maintainers??) can work on some new ones
for -next when that opens.

Once we get to the point where every bootstd series doesn't raise a
discussion about EFI, I will feel a lot more comfortable about
changing defaults. I hope you can understand that...

Regards,
Simon
Tom Rini April 8, 2023, 3:14 p.m. UTC | #7
On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 8 Apr 2023 at 09:55, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > > > >
> > > > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > > > fixes various problems which have come up recently, showing differences
> > > > > > > between the current implementation and the distroboot scripts.
> > > > > > >
> > > > > > > This should get us closer to being able to turn down the scripts.
> > > > > >
> > > > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > > > goal should be that we get to the point where we can "default y if ARM
> > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > > > just a few architectures that haven't ended up being converted. But
> > > > > > today, there's too much churn on platforms that aren't making any use of
> > > > > > this. And I don't think this is going to be functionally worse than all
> > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > > > replace that with "imply BOOTSTD" as they get migrated.
> > > > >
> > > > > That would really be a backward step. I'm not sure what to say at this
> > > > > point. I've put a lot of effort in trying to get this over the line,
> > > > > but the only way we get feedback is when it is applied.
> > > >
> > > > Having bootstd enabled and not functional (because boot_targets aren't
> > > > set) isn't helping the migration happen. And the hard part of the
> > > > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > > > testing it and also it really just being 1-2 options.
> > >
> > > Standard boot does not need boot_targets to be set. It works fine
> > > without it. It just goes through the boot devices in a pre-defined
> > > order, from fastest to slowest. It matches what most boards do anyway.
> > > The main reason we kept it is for compatibility with distro boot.
> >
> > What most boards do today is just sit at the prompt and wait for input,
> > which this changes, which is part of the big source of size churn here.
> 
> Yes.
> 
> >
> > > I don't think testing in advance is a feasible approach in general.
> > > See for example the rpi series which hasn't got any comment. It likely
> > > won't until it is applied. That's how we get feedback. We have months
> > > to resolve issues and I believe that the code is fundamentally sound.
> >
> > We need some spot testing here and there to see how things react and how
> > people use it. You found a lot of things with just rk3399, and now the
> > rest of rockchip looks to be fairly direct. You found more things doing
> > x86. When you can convert some other SoC and the change is just dropping
> > distro_bootcmd and calling bootflow scan instead (or that + setting the
> > order again), that'll be good.
> 
> So long as we are aware that we generally only find problems when
> patches land, yes. The QEMU stuff is easier since it doesn't need a
> board. It's also not all that useful in the real world :-) I'd like to
> try a programmatic conversion, too, although I haven't looked at it
> yet.

Well, I think you're misjudging when we get most of the testing done.
It's at that last one or two -rc points where it seems people test
things, and surprises are very much not welcome then, which is why I
want more unit testing of things like this.  Especially as we're just
getting started on the conversions.

> > But I'm not sure that changing the platforms that don't today opt-in to
> > distro_bootcmd (which has been a thing for a long time now) to force
> > opt-in to this is the right call. It might be in some cases (mediatek
> > maybe? Or maybe no, everyone does android of some flavour so a different
> > bootstd option) but not others (those *_evm_r5_defconfig boards).
> 
> Fair enough, so long as we actually turn down distro_bootcmd. So long
> as it is still there, boards will enable it. See SPL_FIT_GENERATOR.

Yes, it will take follow-through to get everyone converted, and making
sure the new tool covers all the use cases.

> > > > > What churn are you seeing? Do you mean:
> > > > >
> > > > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> > > >
> > > > We need all 3 of those patches because without the 3rd you don't get a
> > > > good experience when you do enable bootstd for a platform. The problem
> > > > is that unless you have distro_bootcmd today you're getting between 63kB
> > > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > > > growth because bootstd is on and now is the default bootcmd, when before
> > > > they had nothing. And probably had board docs saying "now do ... to
> > > > boot". And that's largely setting aside the *_r5_* platforms that I know
> > > > are just doing something else, and could disable it.
> > >
> > > Er, I thought you wanted it to default on if the boards has no
> > > bootcmd? If not, we can disable it for those as well. If you don't
> > > want any increase we can disable it for boards without DISTRO_DEFAULTS
> > > too. After all, presumably those boards are doing something custom
> > > anyway.
> >
> > I think I might have originally, but now that I'm looking at the results
> > it was too optimistic. Every branch I build I look at the per-board
> > breakdown, not just the summary. And it's too much on all of these
> > platforms that had no default bootcmd today.
> 
> OK. I don't mind about that. We've ended up in a bit of a rabbit hole I think.
> 
> >
> > > > We want to convert everyone doing distro_bootcmd over to this, that's
> > > > good. The problem is we don't have a symbol today that means "we want
> > > > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > > > in this sense).
> > > >
> > > > The wrong direction part of this series is that for platforms that
> > > > aren't in the middle of converting we're increasing their size between
> > > > somewhat and very very much, and we haven't tested that it'll work. And
> > > > yes, there's some automatic guessing logic, which hasn't been tested on
> > > > these platforms either, so we don't actually know if going from no
> > > > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > > > improvement.
> > >
> > > So, the wrong direction comes from the last three patches. Is that right?
> >
> > The wrong direction comes from enabling bootstd (and so, a bootcmd and
> > distro boot) on platforms that just sit at the prompt today.  We are not
> > making a good heuristic guess at what the should be doing.  Reaching out
> > to the maintainers to get them to do the conversion, especially once
> > it's just a matter for most of them of just enabling bootstd and then
> > distro, if they want that or once bootmeth android gets done, for those
> > platforms) is how we get them moved smoothly.
> 
> We should know which boards sit at the prompt today, by the fact that
> they don't have a bootcmd. Presumably that is the way the maintainer
> wants it, so I agree we should avoid changing it.

Well, doing:
for C in `(cd configs;ls)`;do make -sj $C && mv .config configs/$C;done
to give me everyones full config to browse, there's 564 boards calling
distro_bootcmd, but 465 of them are direct "run distro_bootcmd" and
nothing more. Those are the ones that'll be easy to migrate, once we've
got another migration or two done.

> > > Fundamentally the problem I have is that I know where I would like
> > > this to head, which is everything using standard boot and turning down
> > > the scripts. But it feels like every time I touch bootstd we have to
> > > have the EFI discussion again. You can imagine how I feel about
> > > disabling BOOTSTD by default...it would basically kill it.
> >
> > Well, we enabled bootstd by default too quickly perhaps then, and just
> > like we narrowed down EFI_LOADER defaults, we need to narrow this down
> > until it's easily convertable.
> 
> It feels like it took a year to get that moving. There was so much EFI
> discussion that I really don't want to revisit.

Yes, and there too I started off with "well, this works for everyone,
so, OK" and then saw that "Well, OK, a lot of things don't need/want
that, really".

> > > This is not really an arch-specific thing, nor an SoC-specific thing.
> > > The underlying logic is the same for everything. The reason I think we
> > > need to do a few cases before we enable it everywhere is that we need
> > > to find the little tweaks needed in that logic.
> >
> > It's a generic framework to a board specific thing.
> >
> > > How about we apply the first patches in this series, skipping the last
> > > three, then apply the rpi series as well. That should get people
> > > actually using it and we can iron out the problems. It also keeps
> > > things moving. We have months before the release.
> > >
> > > Enabling by default can come later once we decide what we want to do
> > > about size increases, boards that don't use DISTRO_DEFAULTS and boards
> > > that don't have a boot command.
> >
> > How about disabling it by default and imply'ing it for everything that
> > implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
> > done is the distro bootmeth?
> 
> I'd really like to move forwards, instead of creating another barrier
> to this migration. There was a huge amount of work in making sure that
> the incremental size of bootstd was small, so it could be cheaply
> enabled. This all went off the rails because, as you correctly pointed
> out, enabling the bootstd commands does ~nothing if there is no actual
> boot command set. I wish I had just stopped then to clarify the goals,
> because this has all burned a lot of time and energy.
> 
> From my side the best thing to do would be to get the currently
> outstanding migrations into -master ASAP so people can try them out.
> Despite the delays we still have months left for testing on this
> release. Then I (or perhaps maintainers??) can work on some new ones
> for -next when that opens.

Yes, we can take the fixes in this series.  And yes, we can take
finishing moving the rest of rockchip over, since I put in the line to
make sure we still grab all of the defaults generic distro needs. And I
am hopeful Peter will have the time to test the Pi conversion on all the
hardware he wants to test it on (along with booting up whatever OS
combinations).

Another one of those big chunk of boards is the sunxi families, if you
want to find some boards and boot some distros.

> Once we get to the point where every bootstd series doesn't raise a
> discussion about EFI, I will feel a lot more comfortable about
> changing defaults. I hope you can understand that...

Well, that's going to be the case until you've handed bootmeth_distro
off to someone else, most likely.  Generic distro boot means EFI boot.
And I only mentioned EFI here as an example of should have pushed back
on "enable this for everyone" sooner.

Because that's the key here, on the 564 platforms that use
distro_bootcmd today, there shouldn't be any growth when we switch to
bootstd, and there's even more platforms that enable
CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the
cases like xilinx mini should be the exception, not the rule.  And
that's what I keep circling back to. The logic to keep the defaults that
generic distro support needs are (almost) always already set on the
platforms that are using generic distro boot, so we shouldn't grow in
size when bootstd turns it on.  And if I'm changing my mind about
forcing this on, on platforms that hadn't been doing generic distro
before, I guess I'm changing my mind, sorry, there's too many platforms
that are doing other things (not generic distro or Android or Special
Things).
Simon Glass April 8, 2023, 8:18 p.m. UTC | #8
Hi Tom,

On Sun, 9 Apr 2023 at 03:14, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 8 Apr 2023 at 09:55, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > > > > >
> > > > > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > > > > fixes various problems which have come up recently, showing differences
> > > > > > > > between the current implementation and the distroboot scripts.
> > > > > > > >
> > > > > > > > This should get us closer to being able to turn down the scripts.
> > > > > > >
> > > > > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > > > > goal should be that we get to the point where we can "default y if ARM
> > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > > > > just a few architectures that haven't ended up being converted. But
> > > > > > > today, there's too much churn on platforms that aren't making any use of
> > > > > > > this. And I don't think this is going to be functionally worse than all
> > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > > > > replace that with "imply BOOTSTD" as they get migrated.
> > > > > >
> > > > > > That would really be a backward step. I'm not sure what to say at this
> > > > > > point. I've put a lot of effort in trying to get this over the line,
> > > > > > but the only way we get feedback is when it is applied.
> > > > >
> > > > > Having bootstd enabled and not functional (because boot_targets aren't
> > > > > set) isn't helping the migration happen. And the hard part of the
> > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > > > > testing it and also it really just being 1-2 options.
> > > >
> > > > Standard boot does not need boot_targets to be set. It works fine
> > > > without it. It just goes through the boot devices in a pre-defined
> > > > order, from fastest to slowest. It matches what most boards do anyway.
> > > > The main reason we kept it is for compatibility with distro boot.
> > >
> > > What most boards do today is just sit at the prompt and wait for input,
> > > which this changes, which is part of the big source of size churn here.
> >
> > Yes.
> >
> > >
> > > > I don't think testing in advance is a feasible approach in general.
> > > > See for example the rpi series which hasn't got any comment. It likely
> > > > won't until it is applied. That's how we get feedback. We have months
> > > > to resolve issues and I believe that the code is fundamentally sound.
> > >
> > > We need some spot testing here and there to see how things react and how
> > > people use it. You found a lot of things with just rk3399, and now the
> > > rest of rockchip looks to be fairly direct. You found more things doing
> > > x86. When you can convert some other SoC and the change is just dropping
> > > distro_bootcmd and calling bootflow scan instead (or that + setting the
> > > order again), that'll be good.
> >
> > So long as we are aware that we generally only find problems when
> > patches land, yes. The QEMU stuff is easier since it doesn't need a
> > board. It's also not all that useful in the real world :-) I'd like to
> > try a programmatic conversion, too, although I haven't looked at it
> > yet.
>
> Well, I think you're misjudging when we get most of the testing done.
> It's at that last one or two -rc points where it seems people test
> things, and surprises are very much not welcome then, which is why I
> want more unit testing of things like this.  Especially as we're just
> getting started on the conversions.

The unit tests are in test/boot and there's quite a bit. But it does
not cover some of the minor details though, e.g. how the fdt file is
selected. It can certainly be added, once we know what the correct
behaviour is. I think it needs more users, before I can tell how close
we are.

The nice thing with bootstd is that we can create unit tests for this
behaviour. That isn't possible with the scripts.

>
> > > But I'm not sure that changing the platforms that don't today opt-in to
> > > distro_bootcmd (which has been a thing for a long time now) to force
> > > opt-in to this is the right call. It might be in some cases (mediatek
> > > maybe? Or maybe no, everyone does android of some flavour so a different
> > > bootstd option) but not others (those *_evm_r5_defconfig boards).
> >
> > Fair enough, so long as we actually turn down distro_bootcmd. So long
> > as it is still there, boards will enable it. See SPL_FIT_GENERATOR.
>
> Yes, it will take follow-through to get everyone converted, and making
> sure the new tool covers all the use cases.
>
> > > > > > What churn are you seeing? Do you mean:
> > > > > >
> > > > > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > > > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> > > > >
> > > > > We need all 3 of those patches because without the 3rd you don't get a
> > > > > good experience when you do enable bootstd for a platform. The problem
> > > > > is that unless you have distro_bootcmd today you're getting between 63kB
> > > > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > > > > growth because bootstd is on and now is the default bootcmd, when before
> > > > > they had nothing. And probably had board docs saying "now do ... to
> > > > > boot". And that's largely setting aside the *_r5_* platforms that I know
> > > > > are just doing something else, and could disable it.
> > > >
> > > > Er, I thought you wanted it to default on if the boards has no
> > > > bootcmd? If not, we can disable it for those as well. If you don't
> > > > want any increase we can disable it for boards without DISTRO_DEFAULTS
> > > > too. After all, presumably those boards are doing something custom
> > > > anyway.
> > >
> > > I think I might have originally, but now that I'm looking at the results
> > > it was too optimistic. Every branch I build I look at the per-board
> > > breakdown, not just the summary. And it's too much on all of these
> > > platforms that had no default bootcmd today.
> >
> > OK. I don't mind about that. We've ended up in a bit of a rabbit hole I think.
> >
> > >
> > > > > We want to convert everyone doing distro_bootcmd over to this, that's
> > > > > good. The problem is we don't have a symbol today that means "we want
> > > > > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > > > > in this sense).
> > > > >
> > > > > The wrong direction part of this series is that for platforms that
> > > > > aren't in the middle of converting we're increasing their size between
> > > > > somewhat and very very much, and we haven't tested that it'll work. And
> > > > > yes, there's some automatic guessing logic, which hasn't been tested on
> > > > > these platforms either, so we don't actually know if going from no
> > > > > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > > > > improvement.
> > > >
> > > > So, the wrong direction comes from the last three patches. Is that right?
> > >
> > > The wrong direction comes from enabling bootstd (and so, a bootcmd and
> > > distro boot) on platforms that just sit at the prompt today.  We are not
> > > making a good heuristic guess at what the should be doing.  Reaching out
> > > to the maintainers to get them to do the conversion, especially once
> > > it's just a matter for most of them of just enabling bootstd and then
> > > distro, if they want that or once bootmeth android gets done, for those
> > > platforms) is how we get them moved smoothly.
> >
> > We should know which boards sit at the prompt today, by the fact that
> > they don't have a bootcmd. Presumably that is the way the maintainer
> > wants it, so I agree we should avoid changing it.
>
> Well, doing:
> for C in `(cd configs;ls)`;do make -sj $C && mv .config configs/$C;done
> to give me everyones full config to browse, there's 564 boards calling
> distro_bootcmd, but 465 of them are direct "run distro_bootcmd" and
> nothing more. Those are the ones that'll be easy to migrate, once we've
> got another migration or two done.

Hmm it would be nice if moveconfig let you summarise values of CONFIG vars.

Some of these will need tweaking, e.g. to replace the custom command
with something that uses bootstd.

>
> > > > Fundamentally the problem I have is that I know where I would like
> > > > this to head, which is everything using standard boot and turning down
> > > > the scripts. But it feels like every time I touch bootstd we have to
> > > > have the EFI discussion again. You can imagine how I feel about
> > > > disabling BOOTSTD by default...it would basically kill it.
> > >
> > > Well, we enabled bootstd by default too quickly perhaps then, and just
> > > like we narrowed down EFI_LOADER defaults, we need to narrow this down
> > > until it's easily convertable.
> >
> > It feels like it took a year to get that moving. There was so much EFI
> > discussion that I really don't want to revisit.
>
> Yes, and there too I started off with "well, this works for everyone,
> so, OK" and then saw that "Well, OK, a lot of things don't need/want
> that, really".
>
> > > > This is not really an arch-specific thing, nor an SoC-specific thing.
> > > > The underlying logic is the same for everything. The reason I think we
> > > > need to do a few cases before we enable it everywhere is that we need
> > > > to find the little tweaks needed in that logic.
> > >
> > > It's a generic framework to a board specific thing.
> > >
> > > > How about we apply the first patches in this series, skipping the last
> > > > three, then apply the rpi series as well. That should get people
> > > > actually using it and we can iron out the problems. It also keeps
> > > > things moving. We have months before the release.
> > > >
> > > > Enabling by default can come later once we decide what we want to do
> > > > about size increases, boards that don't use DISTRO_DEFAULTS and boards
> > > > that don't have a boot command.
> > >
> > > How about disabling it by default and imply'ing it for everything that
> > > implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
> > > done is the distro bootmeth?
> >
> > I'd really like to move forwards, instead of creating another barrier
> > to this migration. There was a huge amount of work in making sure that
> > the incremental size of bootstd was small, so it could be cheaply
> > enabled. This all went off the rails because, as you correctly pointed
> > out, enabling the bootstd commands does ~nothing if there is no actual
> > boot command set. I wish I had just stopped then to clarify the goals,
> > because this has all burned a lot of time and energy.
> >
> > From my side the best thing to do would be to get the currently
> > outstanding migrations into -master ASAP so people can try them out.
> > Despite the delays we still have months left for testing on this
> > release. Then I (or perhaps maintainers??) can work on some new ones
> > for -next when that opens.
>
> Yes, we can take the fixes in this series.  And yes, we can take
> finishing moving the rest of rockchip over, since I put in the line to
> make sure we still grab all of the defaults generic distro needs. And I
> am hopeful Peter will have the time to test the Pi conversion on all the
> hardware he wants to test it on (along with booting up whatever OS
> combinations).
>
> Another one of those big chunk of boards is the sunxi families, if you
> want to find some boards and boot some distros.

OK. Let's get rpi in and I can look at sunxi for -next

>
> > Once we get to the point where every bootstd series doesn't raise a
> > discussion about EFI, I will feel a lot more comfortable about
> > changing defaults. I hope you can understand that...
>
> Well, that's going to be the case until you've handed bootmeth_distro
> off to someone else, most likely.  Generic distro boot means EFI boot.
> And I only mentioned EFI here as an example of should have pushed back
> on "enable this for everyone" sooner.

Would someone like to take it over? Note that bootmeth_distro is just
the extlinux.conf stuff.

>
> Because that's the key here, on the 564 platforms that use
> distro_bootcmd today, there shouldn't be any growth when we switch to
> bootstd, and there's even more platforms that enable
> CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the
> cases like xilinx mini should be the exception, not the rule.  And
> that's what I keep circling back to. The logic to keep the defaults that
> generic distro support needs are (almost) always already set on the
> platforms that are using generic distro boot, so we shouldn't grow in
> size when bootstd turns it on.  And if I'm changing my mind about
> forcing this on, on platforms that hadn't been doing generic distro
> before, I guess I'm changing my mind, sorry, there's too many platforms
> that are doing other things (not generic distro or Android or Special
> Things).

Yes I agree there should be no size growth for platforms which use
distro_bootcmd today and move to bootstd. That is easy enough to check
as part of migration. For the rpi and rockchip series the size reduces
from where we are today by 1-12KB. Re the bootstd default, we should
talk about that.

So basically, the plan for migrations is something like this:
- switch over some group of boards (change from DISTRO_DEFAULTS to
BOOTSTD_DEFAULTS in the process)
- test by booting various distros to see if there are new corner cases
(do we have a list?)
- check there is no size growth
- send patches
- discussion will focus on code review and testing

Does that sound right?

Do you think maintainers might be interested in migrating their boards?

Anyway, I'll resend the series without the last three patches which
generated this discussion.

As above I also see a bit of a gap in testing which would be good to
plug. The bootmeths rely on CI to test with two faked OS images. But
we might be able to add unit tests for these, with a bit of thought. I
will see what I can do.

Regards,
Simon
Tom Rini April 10, 2023, 12:07 a.m. UTC | #9
On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 9 Apr 2023 at 03:14, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 8 Apr 2023 at 09:55, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > > > > > fixes various problems which have come up recently, showing differences
> > > > > > > > > between the current implementation and the distroboot scripts.
> > > > > > > > >
> > > > > > > > > This should get us closer to being able to turn down the scripts.
> > > > > > > >
> > > > > > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > > > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > > > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > > > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > > > > > goal should be that we get to the point where we can "default y if ARM
> > > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > > > > > just a few architectures that haven't ended up being converted. But
> > > > > > > > today, there's too much churn on platforms that aren't making any use of
> > > > > > > > this. And I don't think this is going to be functionally worse than all
> > > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > > > > > replace that with "imply BOOTSTD" as they get migrated.
> > > > > > >
> > > > > > > That would really be a backward step. I'm not sure what to say at this
> > > > > > > point. I've put a lot of effort in trying to get this over the line,
> > > > > > > but the only way we get feedback is when it is applied.
> > > > > >
> > > > > > Having bootstd enabled and not functional (because boot_targets aren't
> > > > > > set) isn't helping the migration happen. And the hard part of the
> > > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > > > > > testing it and also it really just being 1-2 options.
> > > > >
> > > > > Standard boot does not need boot_targets to be set. It works fine
> > > > > without it. It just goes through the boot devices in a pre-defined
> > > > > order, from fastest to slowest. It matches what most boards do anyway.
> > > > > The main reason we kept it is for compatibility with distro boot.
> > > >
> > > > What most boards do today is just sit at the prompt and wait for input,
> > > > which this changes, which is part of the big source of size churn here.
> > >
> > > Yes.
> > >
> > > >
> > > > > I don't think testing in advance is a feasible approach in general.
> > > > > See for example the rpi series which hasn't got any comment. It likely
> > > > > won't until it is applied. That's how we get feedback. We have months
> > > > > to resolve issues and I believe that the code is fundamentally sound.
> > > >
> > > > We need some spot testing here and there to see how things react and how
> > > > people use it. You found a lot of things with just rk3399, and now the
> > > > rest of rockchip looks to be fairly direct. You found more things doing
> > > > x86. When you can convert some other SoC and the change is just dropping
> > > > distro_bootcmd and calling bootflow scan instead (or that + setting the
> > > > order again), that'll be good.
> > >
> > > So long as we are aware that we generally only find problems when
> > > patches land, yes. The QEMU stuff is easier since it doesn't need a
> > > board. It's also not all that useful in the real world :-) I'd like to
> > > try a programmatic conversion, too, although I haven't looked at it
> > > yet.
> >
> > Well, I think you're misjudging when we get most of the testing done.
> > It's at that last one or two -rc points where it seems people test
> > things, and surprises are very much not welcome then, which is why I
> > want more unit testing of things like this.  Especially as we're just
> > getting started on the conversions.
> 
> The unit tests are in test/boot and there's quite a bit. But it does
> not cover some of the minor details though, e.g. how the fdt file is
> selected. It can certainly be added, once we know what the correct
> behaviour is. I think it needs more users, before I can tell how close
> we are.
> 
> The nice thing with bootstd is that we can create unit tests for this
> behaviour. That isn't possible with the scripts.

Yes, writing tests for the code is good.  This is not a substitute for
booting up assorted previously functional OS images / installers on the
platform. That's how we ran in to the rk3399 problems.

> > > > But I'm not sure that changing the platforms that don't today opt-in to
> > > > distro_bootcmd (which has been a thing for a long time now) to force
> > > > opt-in to this is the right call. It might be in some cases (mediatek
> > > > maybe? Or maybe no, everyone does android of some flavour so a different
> > > > bootstd option) but not others (those *_evm_r5_defconfig boards).
> > >
> > > Fair enough, so long as we actually turn down distro_bootcmd. So long
> > > as it is still there, boards will enable it. See SPL_FIT_GENERATOR.
> >
> > Yes, it will take follow-through to get everyone converted, and making
> > sure the new tool covers all the use cases.
> >
> > > > > > > What churn are you seeing? Do you mean:
> > > > > > >
> > > > > > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > > > > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > > > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> > > > > >
> > > > > > We need all 3 of those patches because without the 3rd you don't get a
> > > > > > good experience when you do enable bootstd for a platform. The problem
> > > > > > is that unless you have distro_bootcmd today you're getting between 63kB
> > > > > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > > > > > growth because bootstd is on and now is the default bootcmd, when before
> > > > > > they had nothing. And probably had board docs saying "now do ... to
> > > > > > boot". And that's largely setting aside the *_r5_* platforms that I know
> > > > > > are just doing something else, and could disable it.
> > > > >
> > > > > Er, I thought you wanted it to default on if the boards has no
> > > > > bootcmd? If not, we can disable it for those as well. If you don't
> > > > > want any increase we can disable it for boards without DISTRO_DEFAULTS
> > > > > too. After all, presumably those boards are doing something custom
> > > > > anyway.
> > > >
> > > > I think I might have originally, but now that I'm looking at the results
> > > > it was too optimistic. Every branch I build I look at the per-board
> > > > breakdown, not just the summary. And it's too much on all of these
> > > > platforms that had no default bootcmd today.
> > >
> > > OK. I don't mind about that. We've ended up in a bit of a rabbit hole I think.
> > >
> > > >
> > > > > > We want to convert everyone doing distro_bootcmd over to this, that's
> > > > > > good. The problem is we don't have a symbol today that means "we want
> > > > > > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > > > > > in this sense).
> > > > > >
> > > > > > The wrong direction part of this series is that for platforms that
> > > > > > aren't in the middle of converting we're increasing their size between
> > > > > > somewhat and very very much, and we haven't tested that it'll work. And
> > > > > > yes, there's some automatic guessing logic, which hasn't been tested on
> > > > > > these platforms either, so we don't actually know if going from no
> > > > > > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > > > > > improvement.
> > > > >
> > > > > So, the wrong direction comes from the last three patches. Is that right?
> > > >
> > > > The wrong direction comes from enabling bootstd (and so, a bootcmd and
> > > > distro boot) on platforms that just sit at the prompt today.  We are not
> > > > making a good heuristic guess at what the should be doing.  Reaching out
> > > > to the maintainers to get them to do the conversion, especially once
> > > > it's just a matter for most of them of just enabling bootstd and then
> > > > distro, if they want that or once bootmeth android gets done, for those
> > > > platforms) is how we get them moved smoothly.
> > >
> > > We should know which boards sit at the prompt today, by the fact that
> > > they don't have a bootcmd. Presumably that is the way the maintainer
> > > wants it, so I agree we should avoid changing it.
> >
> > Well, doing:
> > for C in `(cd configs;ls)`;do make -sj $C && mv .config configs/$C;done
> > to give me everyones full config to browse, there's 564 boards calling
> > distro_bootcmd, but 465 of them are direct "run distro_bootcmd" and
> > nothing more. Those are the ones that'll be easy to migrate, once we've
> > got another migration or two done.
> 
> Hmm it would be nice if moveconfig let you summarise values of CONFIG vars.

Could be interesting, could be too much work to bother with, hard to
say.

> Some of these will need tweaking, e.g. to replace the custom command
> with something that uses bootstd.

Well, yes and no?  They'll require looking at the use case and
understanding what's going on. We've got some platforms that need to
make sure we have fdtfile/uuid set (how are we doing that today in the
generic distro bootmeth?) and others are "try distro, then try android",
which means we need the android bootmeth to exist. Others still are a
fall back.

> > > > > Fundamentally the problem I have is that I know where I would like
> > > > > this to head, which is everything using standard boot and turning down
> > > > > the scripts. But it feels like every time I touch bootstd we have to
> > > > > have the EFI discussion again. You can imagine how I feel about
> > > > > disabling BOOTSTD by default...it would basically kill it.
> > > >
> > > > Well, we enabled bootstd by default too quickly perhaps then, and just
> > > > like we narrowed down EFI_LOADER defaults, we need to narrow this down
> > > > until it's easily convertable.
> > >
> > > It feels like it took a year to get that moving. There was so much EFI
> > > discussion that I really don't want to revisit.
> >
> > Yes, and there too I started off with "well, this works for everyone,
> > so, OK" and then saw that "Well, OK, a lot of things don't need/want
> > that, really".
> >
> > > > > This is not really an arch-specific thing, nor an SoC-specific thing.
> > > > > The underlying logic is the same for everything. The reason I think we
> > > > > need to do a few cases before we enable it everywhere is that we need
> > > > > to find the little tweaks needed in that logic.
> > > >
> > > > It's a generic framework to a board specific thing.
> > > >
> > > > > How about we apply the first patches in this series, skipping the last
> > > > > three, then apply the rpi series as well. That should get people
> > > > > actually using it and we can iron out the problems. It also keeps
> > > > > things moving. We have months before the release.
> > > > >
> > > > > Enabling by default can come later once we decide what we want to do
> > > > > about size increases, boards that don't use DISTRO_DEFAULTS and boards
> > > > > that don't have a boot command.
> > > >
> > > > How about disabling it by default and imply'ing it for everything that
> > > > implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
> > > > done is the distro bootmeth?
> > >
> > > I'd really like to move forwards, instead of creating another barrier
> > > to this migration. There was a huge amount of work in making sure that
> > > the incremental size of bootstd was small, so it could be cheaply
> > > enabled. This all went off the rails because, as you correctly pointed
> > > out, enabling the bootstd commands does ~nothing if there is no actual
> > > boot command set. I wish I had just stopped then to clarify the goals,
> > > because this has all burned a lot of time and energy.
> > >
> > > From my side the best thing to do would be to get the currently
> > > outstanding migrations into -master ASAP so people can try them out.
> > > Despite the delays we still have months left for testing on this
> > > release. Then I (or perhaps maintainers??) can work on some new ones
> > > for -next when that opens.
> >
> > Yes, we can take the fixes in this series.  And yes, we can take
> > finishing moving the rest of rockchip over, since I put in the line to
> > make sure we still grab all of the defaults generic distro needs. And I
> > am hopeful Peter will have the time to test the Pi conversion on all the
> > hardware he wants to test it on (along with booting up whatever OS
> > combinations).
> >
> > Another one of those big chunk of boards is the sunxi families, if you
> > want to find some boards and boot some distros.
> 
> OK. Let's get rpi in and I can look at sunxi for -next
> 
> >
> > > Once we get to the point where every bootstd series doesn't raise a
> > > discussion about EFI, I will feel a lot more comfortable about
> > > changing defaults. I hope you can understand that...
> >
> > Well, that's going to be the case until you've handed bootmeth_distro
> > off to someone else, most likely.  Generic distro boot means EFI boot.
> > And I only mentioned EFI here as an example of should have pushed back
> > on "enable this for everyone" sooner.
> 
> Would someone like to take it over? Note that bootmeth_distro is just
> the extlinux.conf stuff.
> 
> >
> > Because that's the key here, on the 564 platforms that use
> > distro_bootcmd today, there shouldn't be any growth when we switch to
> > bootstd, and there's even more platforms that enable
> > CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the
> > cases like xilinx mini should be the exception, not the rule.  And
> > that's what I keep circling back to. The logic to keep the defaults that
> > generic distro support needs are (almost) always already set on the
> > platforms that are using generic distro boot, so we shouldn't grow in
> > size when bootstd turns it on.  And if I'm changing my mind about
> > forcing this on, on platforms that hadn't been doing generic distro
> > before, I guess I'm changing my mind, sorry, there's too many platforms
> > that are doing other things (not generic distro or Android or Special
> > Things).
> 
> Yes I agree there should be no size growth for platforms which use
> distro_bootcmd today and move to bootstd. That is easy enough to check
> as part of migration. For the rpi and rockchip series the size reduces
> from where we are today by 1-12KB.

It shouldn't change it at all tho, let alone reduce? Why is it reducing?

> Re the bootstd default, we should talk about that.

Yes.

> So basically, the plan for migrations is something like this:
> - switch over some group of boards (change from DISTRO_DEFAULTS to
> BOOTSTD_DEFAULTS in the process)

Yes.

> - test by booting various distros to see if there are new corner cases
> (do we have a list?)

Specifically? No, just distrowatch for random ones outside of the big
common ones. We don't need to test everything of course.

> - check there is no size growth
> - send patches
> - discussion will focus on code review and testing
> 
> Does that sound right?

I think so, yes.

> Do you think maintainers might be interested in migrating their boards?

I know some are, yes. But it will also require figuring out what and how
to extend what we have today. Look at configs/j721e_evm_a72_defconfig
which is first run distro boot, if that fails then we have a more
advanced OS and so initialize all of the other procs (remoteproc stuff,
in tree already) and start that. Moving all of that to bootstd is a
want, but I don't know if doc/develop/bootstd.rst is sufficient today
for someone at TI to pick up and run with it, do you?

> Anyway, I'll resend the series without the last three patches which
> generated this discussion.
> 
> As above I also see a bit of a gap in testing which would be good to
> plug. The bootmeths rely on CI to test with two faked OS images. But
> we might be able to add unit tests for these, with a bit of thought. I
> will see what I can do.

OK. And once we have stuff converted that's in my local farm, at least
an occasional don't interrupt and just autoboot to whatever-I-installed
should happen.
Simon Glass April 12, 2023, 11:03 p.m. UTC | #10
Hi Tom,

On Sun, 9 Apr 2023 at 18:07, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sun, 9 Apr 2023 at 03:14, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 8 Apr 2023 at 09:55, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote:
> > > > > > > > >
> > > > > > > > > > This series converts rockchip boards over to use standard boot. It also
> > > > > > > > > > fixes various problems which have come up recently, showing differences
> > > > > > > > > > between the current implementation and the distroboot scripts.
> > > > > > > > > >
> > > > > > > > > > This should get us closer to being able to turn down the scripts.
> > > > > > > > >
> > > > > > > > > Alright, so I grabbed a few parts of this series to investigate the
> > > > > > > > > points I'm trying to grasp better, and I think this is going the wrong
> > > > > > > > > track. We should start off by dropping "default y" from BOOTSTD, and
> > > > > > > > > then start adding "default y if" for SoCs as we convert them. The end
> > > > > > > > > goal should be that we get to the point where we can "default y if ARM
> > > > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's
> > > > > > > > > just a few architectures that haven't ended up being converted. But
> > > > > > > > > today, there's too much churn on platforms that aren't making any use of
> > > > > > > > > this. And I don't think this is going to be functionally worse than all
> > > > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can
> > > > > > > > > replace that with "imply BOOTSTD" as they get migrated.
> > > > > > > >
> > > > > > > > That would really be a backward step. I'm not sure what to say at this
> > > > > > > > point. I've put a lot of effort in trying to get this over the line,
> > > > > > > > but the only way we get feedback is when it is applied.
> > > > > > >
> > > > > > > Having bootstd enabled and not functional (because boot_targets aren't
> > > > > > > set) isn't helping the migration happen. And the hard part of the
> > > > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's
> > > > > > > testing it and also it really just being 1-2 options.
> > > > > >
> > > > > > Standard boot does not need boot_targets to be set. It works fine
> > > > > > without it. It just goes through the boot devices in a pre-defined
> > > > > > order, from fastest to slowest. It matches what most boards do anyway.
> > > > > > The main reason we kept it is for compatibility with distro boot.
> > > > >
> > > > > What most boards do today is just sit at the prompt and wait for input,
> > > > > which this changes, which is part of the big source of size churn here.
> > > >
> > > > Yes.
> > > >
> > > > >
> > > > > > I don't think testing in advance is a feasible approach in general.
> > > > > > See for example the rpi series which hasn't got any comment. It likely
> > > > > > won't until it is applied. That's how we get feedback. We have months
> > > > > > to resolve issues and I believe that the code is fundamentally sound.
> > > > >
> > > > > We need some spot testing here and there to see how things react and how
> > > > > people use it. You found a lot of things with just rk3399, and now the
> > > > > rest of rockchip looks to be fairly direct. You found more things doing
> > > > > x86. When you can convert some other SoC and the change is just dropping
> > > > > distro_bootcmd and calling bootflow scan instead (or that + setting the
> > > > > order again), that'll be good.
> > > >
> > > > So long as we are aware that we generally only find problems when
> > > > patches land, yes. The QEMU stuff is easier since it doesn't need a
> > > > board. It's also not all that useful in the real world :-) I'd like to
> > > > try a programmatic conversion, too, although I haven't looked at it
> > > > yet.
> > >
> > > Well, I think you're misjudging when we get most of the testing done.
> > > It's at that last one or two -rc points where it seems people test
> > > things, and surprises are very much not welcome then, which is why I
> > > want more unit testing of things like this.  Especially as we're just
> > > getting started on the conversions.
> >
> > The unit tests are in test/boot and there's quite a bit. But it does
> > not cover some of the minor details though, e.g. how the fdt file is
> > selected. It can certainly be added, once we know what the correct
> > behaviour is. I think it needs more users, before I can tell how close
> > we are.
> >
> > The nice thing with bootstd is that we can create unit tests for this
> > behaviour. That isn't possible with the scripts.
>
> Yes, writing tests for the code is good.  This is not a substitute for
> booting up assorted previously functional OS images / installers on the
> platform. That's how we ran in to the rk3399 problems.

Yes. Really we should get these into some sort of CI.

>
> > > > > But I'm not sure that changing the platforms that don't today opt-in to
> > > > > distro_bootcmd (which has been a thing for a long time now) to force
> > > > > opt-in to this is the right call. It might be in some cases (mediatek
> > > > > maybe? Or maybe no, everyone does android of some flavour so a different
> > > > > bootstd option) but not others (those *_evm_r5_defconfig boards).
> > > >
> > > > Fair enough, so long as we actually turn down distro_bootcmd. So long
> > > > as it is still there, boards will enable it. See SPL_FIT_GENERATOR.
> > >
> > > Yes, it will take follow-through to get everyone converted, and making
> > > sure the new tool covers all the use cases.
> > >
> > > > > > > > What churn are you seeing? Do you mean:
> > > > > > > >
> > > > > > > > disable BOOTSTD for boards with custom commands? You asked for that patch
> > > > > > > > disabling BOOTSTD_DEFAULTS? You asked for that patch
> > > > > > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like
> > > > > > >
> > > > > > > We need all 3 of those patches because without the 3rd you don't get a
> > > > > > > good experience when you do enable bootstd for a platform. The problem
> > > > > > > is that unless you have distro_bootcmd today you're getting between 63kB
> > > > > > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of
> > > > > > > growth because bootstd is on and now is the default bootcmd, when before
> > > > > > > they had nothing. And probably had board docs saying "now do ... to
> > > > > > > boot". And that's largely setting aside the *_r5_* platforms that I know
> > > > > > > are just doing something else, and could disable it.
> > > > > >
> > > > > > Er, I thought you wanted it to default on if the boards has no
> > > > > > bootcmd? If not, we can disable it for those as well. If you don't
> > > > > > want any increase we can disable it for boards without DISTRO_DEFAULTS
> > > > > > too. After all, presumably those boards are doing something custom
> > > > > > anyway.
> > > > >
> > > > > I think I might have originally, but now that I'm looking at the results
> > > > > it was too optimistic. Every branch I build I look at the per-board
> > > > > breakdown, not just the summary. And it's too much on all of these
> > > > > platforms that had no default bootcmd today.
> > > >
> > > > OK. I don't mind about that. We've ended up in a bit of a rabbit hole I think.
> > > >
> > > > >
> > > > > > > We want to convert everyone doing distro_bootcmd over to this, that's
> > > > > > > good. The problem is we don't have a symbol today that means "we want
> > > > > > > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded
> > > > > > > in this sense).
> > > > > > >
> > > > > > > The wrong direction part of this series is that for platforms that
> > > > > > > aren't in the middle of converting we're increasing their size between
> > > > > > > somewhat and very very much, and we haven't tested that it'll work. And
> > > > > > > yes, there's some automatic guessing logic, which hasn't been tested on
> > > > > > > these platforms either, so we don't actually know if going from no
> > > > > > > bootcmd (and so drops to prompt) to attempts to autoboot something is an
> > > > > > > improvement.
> > > > > >
> > > > > > So, the wrong direction comes from the last three patches. Is that right?
> > > > >
> > > > > The wrong direction comes from enabling bootstd (and so, a bootcmd and
> > > > > distro boot) on platforms that just sit at the prompt today.  We are not
> > > > > making a good heuristic guess at what the should be doing.  Reaching out
> > > > > to the maintainers to get them to do the conversion, especially once
> > > > > it's just a matter for most of them of just enabling bootstd and then
> > > > > distro, if they want that or once bootmeth android gets done, for those
> > > > > platforms) is how we get them moved smoothly.
> > > >
> > > > We should know which boards sit at the prompt today, by the fact that
> > > > they don't have a bootcmd. Presumably that is the way the maintainer
> > > > wants it, so I agree we should avoid changing it.
> > >
> > > Well, doing:
> > > for C in `(cd configs;ls)`;do make -sj $C && mv .config configs/$C;done
> > > to give me everyones full config to browse, there's 564 boards calling
> > > distro_bootcmd, but 465 of them are direct "run distro_bootcmd" and
> > > nothing more. Those are the ones that'll be easy to migrate, once we've
> > > got another migration or two done.
> >
> > Hmm it would be nice if moveconfig let you summarise values of CONFIG vars.
>
> Could be interesting, could be too much work to bother with, hard to
> say.

I'm not sure either, but anyway we are not there yet.

>
> > Some of these will need tweaking, e.g. to replace the custom command
> > with something that uses bootstd.
>
> Well, yes and no?  They'll require looking at the use case and
> understanding what's going on. We've got some platforms that need to
> make sure we have fdtfile/uuid set (how are we doing that today in the
> generic distro bootmeth?) and others are "try distro, then try android",
> which means we need the android bootmeth to exist. Others still are a
> fall back.

Makes sense. I wonder if someone will write an Android bootmeth?

>
> > > > > > Fundamentally the problem I have is that I know where I would like
> > > > > > this to head, which is everything using standard boot and turning down
> > > > > > the scripts. But it feels like every time I touch bootstd we have to
> > > > > > have the EFI discussion again. You can imagine how I feel about
> > > > > > disabling BOOTSTD by default...it would basically kill it.
> > > > >
> > > > > Well, we enabled bootstd by default too quickly perhaps then, and just
> > > > > like we narrowed down EFI_LOADER defaults, we need to narrow this down
> > > > > until it's easily convertable.
> > > >
> > > > It feels like it took a year to get that moving. There was so much EFI
> > > > discussion that I really don't want to revisit.
> > >
> > > Yes, and there too I started off with "well, this works for everyone,
> > > so, OK" and then saw that "Well, OK, a lot of things don't need/want
> > > that, really".
> > >
> > > > > > This is not really an arch-specific thing, nor an SoC-specific thing.
> > > > > > The underlying logic is the same for everything. The reason I think we
> > > > > > need to do a few cases before we enable it everywhere is that we need
> > > > > > to find the little tweaks needed in that logic.
> > > > >
> > > > > It's a generic framework to a board specific thing.
> > > > >
> > > > > > How about we apply the first patches in this series, skipping the last
> > > > > > three, then apply the rpi series as well. That should get people
> > > > > > actually using it and we can iron out the problems. It also keeps
> > > > > > things moving. We have months before the release.
> > > > > >
> > > > > > Enabling by default can come later once we decide what we want to do
> > > > > > about size increases, boards that don't use DISTRO_DEFAULTS and boards
> > > > > > that don't have a boot command.
> > > > >
> > > > > How about disabling it by default and imply'ing it for everything that
> > > > > implies/selects DISTRO_DEFAULTS today, since the part of bootstd that is
> > > > > done is the distro bootmeth?
> > > >
> > > > I'd really like to move forwards, instead of creating another barrier
> > > > to this migration. There was a huge amount of work in making sure that
> > > > the incremental size of bootstd was small, so it could be cheaply
> > > > enabled. This all went off the rails because, as you correctly pointed
> > > > out, enabling the bootstd commands does ~nothing if there is no actual
> > > > boot command set. I wish I had just stopped then to clarify the goals,
> > > > because this has all burned a lot of time and energy.
> > > >
> > > > From my side the best thing to do would be to get the currently
> > > > outstanding migrations into -master ASAP so people can try them out.
> > > > Despite the delays we still have months left for testing on this
> > > > release. Then I (or perhaps maintainers??) can work on some new ones
> > > > for -next when that opens.
> > >
> > > Yes, we can take the fixes in this series.  And yes, we can take
> > > finishing moving the rest of rockchip over, since I put in the line to
> > > make sure we still grab all of the defaults generic distro needs. And I
> > > am hopeful Peter will have the time to test the Pi conversion on all the
> > > hardware he wants to test it on (along with booting up whatever OS
> > > combinations).
> > >
> > > Another one of those big chunk of boards is the sunxi families, if you
> > > want to find some boards and boot some distros.
> >
> > OK. Let's get rpi in and I can look at sunxi for -next
> >
> > >
> > > > Once we get to the point where every bootstd series doesn't raise a
> > > > discussion about EFI, I will feel a lot more comfortable about
> > > > changing defaults. I hope you can understand that...
> > >
> > > Well, that's going to be the case until you've handed bootmeth_distro
> > > off to someone else, most likely.  Generic distro boot means EFI boot.
> > > And I only mentioned EFI here as an example of should have pushed back
> > > on "enable this for everyone" sooner.
> >
> > Would someone like to take it over? Note that bootmeth_distro is just
> > the extlinux.conf stuff.
> >
> > >
> > > Because that's the key here, on the 564 platforms that use
> > > distro_bootcmd today, there shouldn't be any growth when we switch to
> > > bootstd, and there's even more platforms that enable
> > > CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the
> > > cases like xilinx mini should be the exception, not the rule.  And
> > > that's what I keep circling back to. The logic to keep the defaults that
> > > generic distro support needs are (almost) always already set on the
> > > platforms that are using generic distro boot, so we shouldn't grow in
> > > size when bootstd turns it on.  And if I'm changing my mind about
> > > forcing this on, on platforms that hadn't been doing generic distro
> > > before, I guess I'm changing my mind, sorry, there's too many platforms
> > > that are doing other things (not generic distro or Android or Special
> > > Things).
> >
> > Yes I agree there should be no size growth for platforms which use
> > distro_bootcmd today and move to bootstd. That is easy enough to check
> > as part of migration. For the rpi and rockchip series the size reduces
> > from where we are today by 1-12KB.
>
> It shouldn't change it at all tho, let alone reduce? Why is it reducing?



>
> > Re the bootstd default, we should talk about that.
>
> Yes.
>
> > So basically, the plan for migrations is something like this:
> > - switch over some group of boards (change from DISTRO_DEFAULTS to
> > BOOTSTD_DEFAULTS in the process)
>
> Yes.
>
> > - test by booting various distros to see if there are new corner cases
> > (do we have a list?)
>
> Specifically? No, just distrowatch for random ones outside of the big
> common ones. We don't need to test everything of course.

OK

>
> > - check there is no size growth
> > - send patches
> > - discussion will focus on code review and testing
> >
> > Does that sound right?
>
> I think so, yes.

OK good.

>
> > Do you think maintainers might be interested in migrating their boards?
>
> I know some are, yes. But it will also require figuring out what and how
> to extend what we have today. Look at configs/j721e_evm_a72_defconfig
> which is first run distro boot, if that fails then we have a more
> advanced OS and so initialize all of the other procs (remoteproc stuff,
> in tree already) and start that. Moving all of that to bootstd is a
> want, but I don't know if doc/develop/bootstd.rst is sufficient today
> for someone at TI to pick up and run with it, do you?

Oh wow that is really doing a lot of stuff. Unfortunately it is
relying on pieces of the distro boot script. It needs a look.

>
> > Anyway, I'll resend the series without the last three patches which
> > generated this discussion.
> >
> > As above I also see a bit of a gap in testing which would be good to
> > plug. The bootmeths rely on CI to test with two faked OS images. But
> > we might be able to add unit tests for these, with a bit of thought. I
> > will see what I can do.
>
> OK. And once we have stuff converted that's in my local farm, at least
> an occasional don't interrupt and just autoboot to whatever-I-installed
> should happen.

Yes I do that every now and then but I don't have many distros
installed - I think three.

Regards,
Simon
Simon Glass April 19, 2023, 1:45 a.m. UTC | #11
Hi Tom,

On Wed, 12 Apr 2023 at 17:03, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tom,
>
> On Sun, 9 Apr 2023 at 18:07, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote:
> > > Hi Tom,
> > >
[..]

> > > > Because that's the key here, on the 564 platforms that use
> > > > distro_bootcmd today, there shouldn't be any growth when we switch to
> > > > bootstd, and there's even more platforms that enable
> > > > CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the
> > > > cases like xilinx mini should be the exception, not the rule.  And
> > > > that's what I keep circling back to. The logic to keep the defaults that
> > > > generic distro support needs are (almost) always already set on the
> > > > platforms that are using generic distro boot, so we shouldn't grow in
> > > > size when bootstd turns it on.  And if I'm changing my mind about
> > > > forcing this on, on platforms that hadn't been doing generic distro
> > > > before, I guess I'm changing my mind, sorry, there's too many platforms
> > > > that are doing other things (not generic distro or Android or Special
> > > > Things).
> > >
> > > Yes I agree there should be no size growth for platforms which use
> > > distro_bootcmd today and move to bootstd. That is easy enough to check
> > > as part of migration. For the rpi and rockchip series the size reduces
> > > from where we are today by 1-12KB.
> >
> > It shouldn't change it at all tho, let alone reduce? Why is it reducing?

Just on this point, which I missed, dropping the scripts in the
environment reduces the size by 4-5KB depending on the board. Dropping
hush can reduce by quite a lot. Standard boot does not need scripts or
hush.

Regards,
Simon