From d84b9022e584774ac79d9390126054cb5a2a680b Mon Sep 17 00:00:00 2001 From: "A. Cody Schuffelen" Date: Mon, 26 Feb 2024 21:31:05 -0800 Subject: Use AutoSetup for InitBootloaderEnvPartition Bug: b/303674154 Test: launch_cvd Change-Id: If883f96cecf08fe742e7f8c779ce15d1363edf48 --- host/commands/assemble_cvd/boot_config.cc | 226 ++++++++++----------- host/commands/assemble_cvd/boot_config.h | 10 +- host/commands/assemble_cvd/disk/disk.h | 8 +- .../disk/generate_persistent_vbmeta.cpp | 12 +- host/commands/assemble_cvd/disk_flags.cc | 4 +- 5 files changed, 118 insertions(+), 142 deletions(-) diff --git a/host/commands/assemble_cvd/boot_config.cc b/host/commands/assemble_cvd/boot_config.cc index b1215173c..9604653df 100644 --- a/host/commands/assemble_cvd/boot_config.cc +++ b/host/commands/assemble_cvd/boot_config.cc @@ -133,138 +133,118 @@ size_t WriteEnvironment(const CuttlefishConfig::InstanceSpecific& instance, return env_str.length(); } -} // namespace +std::unordered_map ReplaceKernelBootArgs( + const std::unordered_map& args) { + std::unordered_map ret; + std::transform(std::begin(args), std::end(args), + std::inserter(ret, ret.end()), [](const auto& kv) { + const auto& k = kv.first; + const auto& v = kv.second; + return std::make_pair( + android::base::StringReplace(k, " kernel.", " ", true), + v); + }); + return ret; +} -class InitBootloaderEnvPartitionImpl : public InitBootloaderEnvPartition { - public: - INJECT(InitBootloaderEnvPartitionImpl( - const CuttlefishConfig& config, - const CuttlefishConfig::InstanceSpecific& instance)) - : config_(config), instance_(instance) {} - - // SetupFeature - std::string Name() const override { return "InitBootloaderEnvPartitionImpl"; } - bool Enabled() const override { return !instance_.protected_vm(); } - - private: - std::unordered_set Dependencies() const override { return {}; } - Result ResultSetup() override { - if (instance_.ap_boot_flow() == CuttlefishConfig::InstanceSpecific::APBootFlow::Grub) { - CF_EXPECT(PrepareBootEnvImage( - instance_.ap_uboot_env_image_path(), - CuttlefishConfig::InstanceSpecific::BootFlow::Linux)); - } - CF_EXPECT(PrepareBootEnvImage(instance_.uboot_env_image_path(), - instance_.boot_flow())); +Result PrepareBootEnvImage( + const CuttlefishConfig& config, + const CuttlefishConfig::InstanceSpecific& instance, + const std::string& image_path, + const CuttlefishConfig::InstanceSpecific::BootFlow& flow) { + if (instance.protected_vm()) { return {}; } + auto tmp_boot_env_image_path = image_path + ".tmp"; + auto uboot_env_path = instance.PerInstancePath("mkenvimg_input"); + auto kernel_cmdline = + android::base::Join(KernelCommandLineFromConfig(config, instance), " "); + // If the bootconfig isn't supported in the guest kernel, the bootconfig + // args need to be passed in via the uboot env. This won't be an issue for + // protect kvm which is running a kernel with bootconfig support. + if (!instance.bootconfig_supported()) { + auto bootconfig_args = + CF_EXPECT(BootconfigArgsFromConfig(config, instance)); + + // "androidboot.hardware" kernel parameter has changed to "hardware" in + // bootconfig and needs to be replaced before being used in the kernel + // cmdline. + auto bootconfig_hardware_it = bootconfig_args.find("hardware"); + if (bootconfig_hardware_it != bootconfig_args.end()) { + bootconfig_args["androidboot.hardware"] = bootconfig_hardware_it->second; + bootconfig_args.erase(bootconfig_hardware_it); + } - std::unordered_map ReplaceKernelBootArgs( - const std::unordered_map& args) { - std::unordered_map ret; - std::transform(std::begin(args), std::end(args), - std::inserter(ret, ret.end()), [](const auto& kv) { - const auto& k = kv.first; - const auto& v = kv.second; - return std::make_pair( - android::base::StringReplace(k, " kernel.", " ", true), - v); - }); - return ret; + // TODO(b/182417593): Until we pass the module parameters through + // modules.options, we pass them through bootconfig using + // 'kernel.=' But if we don't support bootconfig, we need to + // rename them back to the old cmdline version + bootconfig_args = ReplaceKernelBootArgs(bootconfig_args); + + kernel_cmdline += + " " + CF_EXPECT(BootconfigArgsString(bootconfig_args, " ")); } - Result PrepareBootEnvImage( - const std::string& image_path, - const CuttlefishConfig::InstanceSpecific::BootFlow& flow) { - auto tmp_boot_env_image_path = image_path + ".tmp"; - auto uboot_env_path = instance_.PerInstancePath("mkenvimg_input"); - auto kernel_cmdline = android::base::Join( - KernelCommandLineFromConfig(config_, instance_), " "); - // If the bootconfig isn't supported in the guest kernel, the bootconfig - // args need to be passed in via the uboot env. This won't be an issue for - // protect kvm which is running a kernel with bootconfig support. - if (!instance_.bootconfig_supported()) { - auto bootconfig_args = - CF_EXPECT(BootconfigArgsFromConfig(config_, instance_)); - - // "androidboot.hardware" kernel parameter has changed to "hardware" in - // bootconfig and needs to be replaced before being used in the kernel - // cmdline. - auto bootconfig_hardware_it = bootconfig_args.find("hardware"); - if (bootconfig_hardware_it != bootconfig_args.end()) { - bootconfig_args["androidboot.hardware"] = - bootconfig_hardware_it->second; - bootconfig_args.erase(bootconfig_hardware_it); - } - - // TODO(b/182417593): Until we pass the module parameters through - // modules.options, we pass them through bootconfig using - // 'kernel.=' But if we don't support bootconfig, we need to - // rename them back to the old cmdline version - bootconfig_args = ReplaceKernelBootArgs(bootconfig_args); - - kernel_cmdline += - " " + CF_EXPECT(BootconfigArgsString(bootconfig_args, " ")); - } + CF_EXPECTF(WriteEnvironment(instance, flow, kernel_cmdline, uboot_env_path), + "Unable to write out plaintext env '{}'", uboot_env_path); + + auto mkimage_path = HostBinaryPath("mkenvimage_slim"); + Command cmd(mkimage_path); + cmd.AddParameter("-output_path"); + cmd.AddParameter(tmp_boot_env_image_path); + cmd.AddParameter("-input_path"); + cmd.AddParameter(uboot_env_path); + int success = cmd.Start().Wait(); + CF_EXPECTF(success == 0, + "Unable to run mkenvimage_slim. Exited with status {}", success); + + const off_t boot_env_size_bytes = + AlignToPowerOf2(MAX_AVB_METADATA_SIZE + 4096, PARTITION_SIZE_SHIFT); + + auto avbtool_path = HostBinaryPath("avbtool"); + Command boot_env_hash_footer_cmd(avbtool_path); + boot_env_hash_footer_cmd.AddParameter("add_hash_footer"); + boot_env_hash_footer_cmd.AddParameter("--image"); + boot_env_hash_footer_cmd.AddParameter(tmp_boot_env_image_path); + boot_env_hash_footer_cmd.AddParameter("--partition_size"); + boot_env_hash_footer_cmd.AddParameter(boot_env_size_bytes); + boot_env_hash_footer_cmd.AddParameter("--partition_name"); + boot_env_hash_footer_cmd.AddParameter("uboot_env"); + boot_env_hash_footer_cmd.AddParameter("--key"); + boot_env_hash_footer_cmd.AddParameter( + DefaultHostArtifactsPath("etc/cvd_avb_testkey.pem")); + boot_env_hash_footer_cmd.AddParameter("--algorithm"); + boot_env_hash_footer_cmd.AddParameter("SHA256_RSA4096"); + success = boot_env_hash_footer_cmd.Start().Wait(); + CF_EXPECTF(success == 0, + "Unable to append hash footer. Exited with status {}", success); + + if (!FileExists(image_path) || + ReadFile(image_path) != ReadFile(tmp_boot_env_image_path)) { + CF_EXPECT(RenameFile(tmp_boot_env_image_path, image_path), + "Unable to delete the old env image"); + LOG(DEBUG) << "Updated bootloader environment image."; + } else { + RemoveFile(tmp_boot_env_image_path); + } - CF_EXPECTF( - WriteEnvironment(instance_, flow, kernel_cmdline, uboot_env_path), - "Unable to write out plaintext env '{}'", uboot_env_path); - - auto mkimage_path = HostBinaryPath("mkenvimage_slim"); - Command cmd(mkimage_path); - cmd.AddParameter("-output_path"); - cmd.AddParameter(tmp_boot_env_image_path); - cmd.AddParameter("-input_path"); - cmd.AddParameter(uboot_env_path); - int success = cmd.Start().Wait(); - CF_EXPECTF(success == 0, - "Unable to run mkenvimage_slim. Exited with status {}", success); - - const off_t boot_env_size_bytes = AlignToPowerOf2( - MAX_AVB_METADATA_SIZE + 4096, PARTITION_SIZE_SHIFT); - - auto avbtool_path = HostBinaryPath("avbtool"); - Command boot_env_hash_footer_cmd(avbtool_path); - boot_env_hash_footer_cmd.AddParameter("add_hash_footer"); - boot_env_hash_footer_cmd.AddParameter("--image"); - boot_env_hash_footer_cmd.AddParameter(tmp_boot_env_image_path); - boot_env_hash_footer_cmd.AddParameter("--partition_size"); - boot_env_hash_footer_cmd.AddParameter(boot_env_size_bytes); - boot_env_hash_footer_cmd.AddParameter("--partition_name"); - boot_env_hash_footer_cmd.AddParameter("uboot_env"); - boot_env_hash_footer_cmd.AddParameter("--key"); - boot_env_hash_footer_cmd.AddParameter( - DefaultHostArtifactsPath("etc/cvd_avb_testkey.pem")); - boot_env_hash_footer_cmd.AddParameter("--algorithm"); - boot_env_hash_footer_cmd.AddParameter("SHA256_RSA4096"); - success = boot_env_hash_footer_cmd.Start().Wait(); - CF_EXPECTF(success == 0, - "Unable to append hash footer. Exited with status {}", success); - - if (!FileExists(image_path) || - ReadFile(image_path) != ReadFile(tmp_boot_env_image_path)) { - CF_EXPECT(RenameFile(tmp_boot_env_image_path, image_path), - "Unable to delete the old env image"); - LOG(DEBUG) << "Updated bootloader environment image."; - } else { - RemoveFile(tmp_boot_env_image_path); - } + return {}; +} - return {}; - } +} // namespace - const CuttlefishConfig& config_; - const CuttlefishConfig::InstanceSpecific& instance_; -}; - -fruit::Component, - InitBootloaderEnvPartition> -InitBootloaderEnvPartitionComponent() { - return fruit::createComponent() - .bind() - .addMultibinding(); +Result InitBootloaderEnvPartition( + const CuttlefishConfig& config, + const CuttlefishConfig::InstanceSpecific& instance) { + if (instance.ap_boot_flow() == + CuttlefishConfig::InstanceSpecific::APBootFlow::Grub) { + CF_EXPECT(PrepareBootEnvImage( + config, instance, instance.ap_uboot_env_image_path(), + CuttlefishConfig::InstanceSpecific::BootFlow::Linux)); + } + CF_EXPECT(PrepareBootEnvImage( + config, instance, instance.uboot_env_image_path(), instance.boot_flow())); + return {}; } } // namespace cuttlefish diff --git a/host/commands/assemble_cvd/boot_config.h b/host/commands/assemble_cvd/boot_config.h index 87318495e..dc5328ff2 100644 --- a/host/commands/assemble_cvd/boot_config.h +++ b/host/commands/assemble_cvd/boot_config.h @@ -17,16 +17,12 @@ #include +#include "common/libs/utils/result.h" #include "host/libs/config/cuttlefish_config.h" -#include "host/libs/config/feature.h" namespace cuttlefish { -class InitBootloaderEnvPartition : public SetupFeature {}; - -fruit::Component, - InitBootloaderEnvPartition> -InitBootloaderEnvPartitionComponent(); +Result InitBootloaderEnvPartition( + const CuttlefishConfig&, const CuttlefishConfig::InstanceSpecific&); } // namespace cuttlefish diff --git a/host/commands/assemble_cvd/disk/disk.h b/host/commands/assemble_cvd/disk/disk.h index 1265d8121..8a94bc2da 100644 --- a/host/commands/assemble_cvd/disk/disk.h +++ b/host/commands/assemble_cvd/disk/disk.h @@ -45,10 +45,10 @@ Gem5ImageUnpackerComponent(); class GeneratePersistentVbmeta : public SetupFeature {}; -fruit::Component< - fruit::Required, - GeneratePersistentVbmeta> +fruit::Component::Type, + GeneratePersistentBootconfig>, + GeneratePersistentVbmeta> GeneratePersistentVbmetaComponent(); class InitializeFactoryResetProtected : public SetupFeature {}; diff --git a/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp b/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp index 1f95b9b1c..a1c4cf2b3 100644 --- a/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp +++ b/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp @@ -34,7 +34,7 @@ class GeneratePersistentVbmetaImpl : public GeneratePersistentVbmeta { public: INJECT(GeneratePersistentVbmetaImpl( const CuttlefishConfig::InstanceSpecific& instance, - InitBootloaderEnvPartition& bootloader_env, + AutoSetup::Type& bootloader_env, GeneratePersistentBootconfig& bootconfig)) : instance_(instance), bootloader_env_(bootloader_env), @@ -111,14 +111,14 @@ class GeneratePersistentVbmetaImpl : public GeneratePersistentVbmeta { } const CuttlefishConfig::InstanceSpecific& instance_; - InitBootloaderEnvPartition& bootloader_env_; + AutoSetup::Type& bootloader_env_; GeneratePersistentBootconfig& bootconfig_; }; -fruit::Component< - fruit::Required, - GeneratePersistentVbmeta> +fruit::Component::Type, + GeneratePersistentBootconfig>, + GeneratePersistentVbmeta> GeneratePersistentVbmetaComponent() { return fruit::createComponent() .addMultibinding() diff --git a/host/commands/assemble_cvd/disk_flags.cc b/host/commands/assemble_cvd/disk_flags.cc index f321def87..3c5c0b151 100644 --- a/host/commands/assemble_cvd/disk_flags.cc +++ b/host/commands/assemble_cvd/disk_flags.cc @@ -684,6 +684,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent( .bindInstance(*config) .bindInstance(*instance) .install(AutoSetup::Component) + .install(AutoSetup::Component) .install(AutoSetup::Component) .install(AutoSetup::Component) .install(AutoSetup::Component) @@ -691,8 +692,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent( .install(GeneratePersistentBootconfigComponent) .install(GeneratePersistentVbmetaComponent) .install(AutoSetup::Component) - .install(InitializeDataImageComponent) - .install(InitBootloaderEnvPartitionComponent); + .install(InitializeDataImageComponent); } Result DiskImageFlagsVectorization(CuttlefishConfig& config, const FetcherConfig& fetcher_config) { -- cgit v1.2.3