Adds a "gpu_mode_candidates" opt for restricting allowed modes by jmacnak · Pull Request #2229 · google/android-cuttlefish

jmacnak · 2026-03-06T21:04:42Z

... and restructures the --gpu_mode=auto selection flow to be

get initial candidates
filter candidates based on requirements
select best remaining candidates

This also has the benefit of allowing the launcher to provide more explicit messaging on a per requirement basis.

Bug: b/489817289
Test: cvd create
Test: cvd create --gpu_mode=gfxstream
Test: cvd create --gpu_mode=gfxstream_guest_angle
Test: cvd create --gpu_mode=gfxstream_guest_angle_host_swiftshader

... as this is no longer needed now that the GpuMode enum from google@82b7702 google@827906f is used. Bug: b/489817289 Test: build

... and restructures the --gpu_mode=auto selection flow to be 1. get initial candidates 2. filter candidates based on requirements 3. select best remaining candidates This also has the benefit of allowing the launcher to provide more explicit messaging on a per requirement basis. Bug: b/489817289 Test: bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all Test: bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all \ --gpu_mode=gfxstream Test: bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all \ --gpu_mode=gfxstream_guest_angle Test: bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all \ --gpu_mode=gfxstream_guest_angle_host_swiftshader Test: bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all \ --gpu_mode=guest_swiftshader Test: <edit android-info.txt in android repo> bazel run //cuttlefish/package:cvd -- \ create \ --host_substitutions=all <observe changed candidates>

Databean · 2026-03-09T23:19:08Z

base/cvd/cuttlefish/host/commands/assemble_cvd/proto/guest_config.proto

+  AUTO = 0;
+  CUSTOM = 1;
+  DRM_VIRGL = 2;
+  GFXSTREAM = 3;
+  GFXSTREAM_GUEST_ANGLE = 4;
+  GFXSTREAM_GUEST_ANGLE_HOST_LAVAPIPE = 5;
+  GFXSTREAM_GUEST_ANGLE_HOST_SWIFTSHADER = 6;
+  GUEST_SWIFTSHADER = 7;
+  NONE = 8;


nit: proto enums should be prefixed with the enum name, i.e. GPU_MODE_AUTO:

https://protobuf.dev/programming-guides/style/#enums

Databean · 2026-03-09T23:26:42Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+bool GpuModeRequirementsMet(const CommonState& common, const GpuMode gpu_mode) {
+  const auto requirements_it = kGpuModeRequirements.find(gpu_mode);
+  CHECK(requirements_it != kGpuModeRequirements.end())
+      << "Failed to find requirements for mode " << GpuModeString(gpu_mode);


It would be convenient to define an AbseilStringify implementation for this enum so that CHECK and LOG can report it directly.

A format_as function would also allow using it with CF_EXPECTF.

Databean · 2026-03-09T23:29:26Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+                RequirementWithReason{
+                    .func = HostIsNotArm,
+                    .success_explanation = "The host is not ARM64.",
+                    .failure_explanation =
+                        "The host is ARM64. Not enabling Gfxstream on ARM64 "
+                        "until vhost-user-gpu "
+                        "has been more thoroughly tested. Please explicitly "
+                        "use "
+                        "--gpu_mode=gfxstream or "
+                        "--gpu_mode=gfxstream_guest_angle to enable "
+                        "for now.",
+                },


Some of these RequirementWithReason instances look duplicated between multiple GpuModes. Is it possible to declare these as named constants and reuse them between multiple std::vector<RequirementWithReason>s?

It might also make it easier to follow if the func implementations are defined closer to the RequirementWithReason instantiations. Alternatively they could also be replaced with anonymous functions if the RequirementWithReason instantiations are deduplicated anyway.

In case there are initialization order issues, it might be more reliable to define functions to return these values, e.g.

RequirementWithReason GuestSupportsGfxstream() { return RequirementWithReason { .func = [](const CommonState& common) { return common.guest_config.gfxstream_supported; }, .success_explanation = "The guest supports Gfxstream.", .failure_explanation = "...", }; }

Databean · 2026-03-09T23:33:25Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+}
+
+bool IsLikelySoftwareRenderer(const std::string& renderer) {
+  const std::string lower_renderer = ToLower(renderer);


Can this use absl::AsciiStrToLower?

Databean · 2026-03-09T23:35:16Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+  std::string success_explanation;
+  std::string failure_explanation;


Can these be std::string_views since they're all being initialized to constants?

Databean · 2026-03-10T00:31:24Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

 #else
  const GpuMode gpu_mode =
-      SelectGpuMode(gpu_mode_arg, vmm, guest_config, graphics_availability);
+      SelectGpuMode(gpu_mode, vmm, guest_config, graphics_availability);


Should this be SelectGpuMode(gpu_mode_arg, ...? This is the statement that declares the gpu_mode variable.

Databean · 2026-03-10T00:35:18Z

base/cvd/cuttlefish/host/commands/assemble_cvd/guest_config.cc

                        {config::DeviceType::Minidroid, DeviceType::Minidroid},
                        {config::DeviceType::Go, DeviceType::Go}};

+  static const std::unordered_map<config::GpuMode, GpuMode> kGpuModeMap = {


Can this use absl::NoDestructor or be dynamically allocated?

See the style guide section on Static and Global Variables

Note that references are not objects, and thus they are not subject to the constraints on destructibility. The constraint on dynamic initialization still applies, though. In particular, a function-local static reference of the form static T& t = *new T; is allowed.

The *new ... strategy might be easier within function scope.

Databean · 2026-03-10T00:35:32Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+  std::string failure_explanation;
+};
+
+const std::unordered_map<GpuMode, std::vector<RequirementWithReason>>


Can this use absl::NoDestructor or be dynamically allocated?

See the style guide section on Static and Global Variables

Note that references are not objects, and thus they are not subject to the constraints on destructibility. The constraint on dynamic initialization still applies, though. In particular, a function-local static reference of the form static T& t = *new T; is allowed.

Databean · 2026-03-10T00:46:10Z

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

+  CHECK(requirements_it != kGpuModeRequirements.end())
+      << "Failed to find requirements for mode " << GpuModeString(gpu_mode);


I'm personally in favor of using Results even to cover "should never happen" cases related to implementation errors, but it's possible to make the case either way. For CHECK:

"Should never happen" intent is clearer.

Function signatures are simpler.

For Result:

Captures the call stack via CF_EXPECT chains without needing to capture it via gdb.

Gives callers the opportunity to do their own cleanup, rather than abort-ing immediately.

Databean · 2026-03-10T00:47:32Z

base/cvd/cuttlefish/host/libs/config/cuttlefish_config_instance.cpp

+  CHECK(json_candidates.isArray())
+      << "Unexpected type for 'gpu_mode_candidates'.";


Same concern as other comment on CHECK.

Remove RenderingMode enum

f1151e3

... as this is no longer needed now that the GpuMode enum from google@82b7702 google@827906f is used. Bug: b/489817289 Test: build

jmacnak force-pushed the gpu-candidates branch from 7c1c270 to a69fb76 Compare March 9, 2026 22:03

jmacnak force-pushed the gpu-candidates branch from a69fb76 to eba67cb Compare March 9, 2026 22:04

jmacnak requested a review from Databean March 9, 2026 23:16

Databean approved these changes Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a "gpu_mode_candidates" opt for restricting allowed modes#2229

Adds a "gpu_mode_candidates" opt for restricting allowed modes#2229
jmacnak wants to merge 2 commits intogoogle:mainfrom
jmacnak:gpu-candidates

jmacnak commented Mar 6, 2026

Uh oh!

Databean Mar 9, 2026

Uh oh!

Databean Mar 9, 2026

Uh oh!

Databean Mar 9, 2026

Uh oh!

Databean Mar 9, 2026

Uh oh!

Databean Mar 9, 2026

Uh oh!

Databean Mar 10, 2026

Uh oh!

Databean Mar 10, 2026

Uh oh!

Databean Mar 10, 2026

Uh oh!

Databean Mar 10, 2026

Uh oh!

Databean Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		std::string success_explanation;
		std::string failure_explanation;

		CHECK(requirements_it != kGpuModeRequirements.end())
		<< "Failed to find requirements for mode " << GpuModeString(gpu_mode);

		CHECK(json_candidates.isArray())
		<< "Unexpected type for 'gpu_mode_candidates'.";

Conversation

jmacnak commented Mar 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants