Attention is currently required from: Gwendal Grignou, Paul Menzel, Paz Zcharya.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email )
Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ......................................................................
Patch Set 7:
(4 comments)
File src/vendorcode/google/chromeos/tpm_factory_config.c:
https://review.coreboot.org/c/coreboot/+/80738/comment/a8538106_347b0be1 : PS7, Line 49: * - Bits 3-0 (0x1): Must be 0x1 to signify compliance with Chromebook-Plus
Not true: this field contains the "feature_level" that indicate the chromebook plus generation. Today, 0 means unknown (may be soft-branded or not chromebook plus), 1 **and above** means chromebook plus.
See go/cros-tiering-dd.
The field name for [bit 0-3] is `hw_x_compliance_version`. I'm using that as a reference
``` hw_x_compliance_version Values: 0x0 : Not CBX compliant 0x1 : Compliant with CBX v1 other: invalid ```
I don't follow what is missing here ?
marking it resolved unless u see something needs to be done.
https://review.coreboot.org/c/coreboot/+/80738/comment/89071c0a_d89384a3 : PS7, Line 74: * requirement.
Not true. A soft branded CBX (crota for instance) will not have any bit set in its GSC but be a CBX (soft branded) device (unless is it re-manufactured). See a crota360, soft branded:
crossystem hwid CROTA360-QYFB D5B-E6Q-D4D-B5M-W9Q-A5A-A9V gsctool -ay raw value: 0000000000000000 chassis_x_branded: false hw_x_compliance_version: 00 feature_check --feature_level 1
crota360 is a legacy device (manufactured before H2'23) hence, we don't select CONFIG_FW_SPLASH_SCREEN config rather we asked those devices to select below config
``` CONFIG_BMP_LOGO=y CONFIG_FSP2_0_LOGO_FILE ```
please refer to the design doc. this code is not even applicable for legacy devices.
marking this comment resolve.
https://review.coreboot.org/c/coreboot/+/80738/comment/889a3228_7ea7cf1c : PS7, Line 97: if (!chromeos_device_plus_hw_compliant())
Most/All soft branded CBX device will not get the proper splash screen with this logic.
note, this logic is only applicable for modern devices (manufactured after H2'23), all older devices will have legacy config enabled hence, this code flow is not applicable for those devices.
All modern devices will be able to meet this requirement.
https://review.coreboot.org/c/coreboot/+/80738/comment/1ca63a2f_0c692bc6 : PS7, Line 104: return strncmp(vpd_get_feature_device_info(), "CAI", 3) == 0;
This is not acceptable for production code. "CAI" is not defined anywhere in the code base. The proper way is to decode the content of the key with `base64decode()`, convert protobuf defined at https://chromium.googlesource.com/chromiumos/platform/feature-management/+/r... and read the `feature_level` field.
softbranded CBX is a time being solution and not something that is long term. GSC bit parsing is a more long term solution hence, CAI still solves the purpose of being able to decode the device branding for those soft-branded devices.
Parsing the protobuf is too much for coreboot like minimalist boot-firmware and not worthy because anyway if protobuf changes in future then eventually we can bring the new string checker alongside `CAI` in future.
Also, after the power wash, the VPD entry is wiped out, so at first boot, the user on soft branded CBX will see the regular `cb_logo.bmp' instead of the expected CBX logo.
Note: this logic is only for soft-branded CBX devices and to my understanding on the first boot (after powerwash, or normal<>dev switch) the plus screens won't appear,but subsequent boots should be fine. This is WAI for the SB devices and no relation with this CL.
marking this resolved as well.